Fix possible use after free in mrb_class_find_path

[lopopolo]

mrb_class_find_path resolves a char* pointer to a class name string
by calling mrb_class_name. It then allocates a new string with
capacity 40 to copy that char* into.

mruby/src/variable.c

Lines 1111 to 1112 in e041841

	str = mrb_class_name(mrb, outer);
	path = mrb_str_new_capa(mrb, 40);

mrb_class_name resolves the class name via class_name_str, which
returns an mrb_value with type tag MRB_TT_STRING and backed by an
RString*. Then mrb_class_name extracts the RSTRING_PTR:

mruby/src/class.c

Lines 2133 to 2134 in e041841

	name = class_name_str(mrb, c);
	return RSTRING_PTR(name);

That RString*-backed mrb_value ultimately comes from mrb_class_path
which resolves the string from the symbol table:

mruby/src/class.c

Line 2111 in e041841

return mrb_sym_str(mrb, mrb_symbol(path));

The allocation of the target str after resolving the class name
mrb_value and extracting its pointer is fragile and assumes the
RString* is "static". If the RString* is not static, the
interleaving of extracting the RSTRING_PTR followed by a subsequent
allocation might result in the class name mrb_value being garbage
collected, which will leave the extracted pointer invalid.

Fix this bad interleaving by allocating the destination string first
before taking a raw pointer to an RString*.

[lopopolo]

I believe this is exploitable because mrb_str_dup is a plausible path to get an RSTRING_PTR which will always be heap allocated:

mruby/src/class.c

Lines 2111 to 2114 in e041841

	return mrb_sym_str(mrb, mrb_symbol(path));
	}
	return mrb_str_dup(mrb, path);
	}

[matz]

Thank you for finding this issue!

[lopopolo]

@matz I don't think I actually fixed this issue. Now if the call mrb_str_dup causes a GC then the previously allocated buffer with capa = 40 might be GC'd.

I think the proper fix is to set the arena index at the beginning of mrb_class_find_path. What do you think?

[dearblue]

I can't imagine a situation where a string pointer is invalidated by the occurrence of a GC.

Unless it is an "inline symbol", the string pointer to the symbol comes from the ROM section or is allocated in the heap which is kept until mrb_close().
Therefore, I find it hard to believe that the string object created by mrb_sym_str() called by mrb_str_new_static() is destroyed, resulting in the string pointer also becoming invalid.

Also, new string objects created by mrb_str_new_static() are registered in the GC arena by mrb_obj_alloc().
So I also don't understand why the object is destroyed in the middle of processing.
The same goes for mrb_str_dup().

Am I missing something?