Skip to content

Commit

Permalink
Fix SEGV by stack extension in mrb_get_args()
Browse files Browse the repository at this point in the history 
mrb_get_args() keeps pointer of the current stack. But address of the
current stack maybe changed by method call.

'i' format character calls #to_i when the argument isn't integer but
has #to_i.

Here is a code that may call #to_i in mrb_get_args():

    case 'i':
      // ...
            default:
              *p = mrb_fixnum(mrb_Integer(mrb, ARGV[arg_i]));
              break;
     // ...

Here is a code #to_i is called:

    class X
      def initialize(i)
        @i = i
      end

      def to_i
        @i
      end
    end

    [][X.new(0), 0] # X#to_i is called

So, mrb_get_args() shouldn't keep pointer and use it. mrb_get_args()
should always refer mrb->ci->stack to use valid address of the current
stack.
  • Loading branch information
@kou
kou committed Jan 21, 2016
1 parent 8a74e68 commit c77123d
Showing 1 changed file with 45 additions and 35 deletions.
80 changes: 45 additions & 35 deletions src/class.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,10 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
{
char c;
int i = 0;
mrb_value *sp = mrb->c->stack + 1;
va_list ap;
int argc = mrb->c->ci->argc;
int arg_i = 0;
mrb_bool array_argv;
mrb_bool opt = FALSE;
mrb_bool given = TRUE;

Expand All @@ -477,8 +478,14 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
struct RArray *a = mrb_ary_ptr(mrb->c->stack[1]);

argc = a->len;
sp = a->ptr;
array_argv = TRUE;
} else {
array_argv = FALSE;
}

#define ARGV \
(array_argv ? mrb_ary_ptr(mrb->c->stack[1])->ptr : (mrb->c->stack + 1))

while ((c = *format++)) {
switch (c) {
case '|': case '*': case '&': case '?':
Expand All @@ -502,7 +509,7 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)

p = va_arg(ap, mrb_value*);
if (i < argc) {
*p = *sp++;
*p = ARGV[arg_i++];
i++;
}
}
Expand All @@ -515,7 +522,7 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
if (i < argc) {
mrb_value ss;

ss = *sp++;
ss = ARGV[arg_i++];
switch (mrb_type(ss)) {
case MRB_TT_CLASS:
case MRB_TT_MODULE:
Expand All @@ -537,14 +544,14 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
p = va_arg(ap, mrb_value*);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
*p = *sp++;
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*p = ARGV[arg_i++];
i++;
break;
}
}
if (i < argc) {
*p = to_str(mrb, *sp++);
*p = to_str(mrb, ARGV[arg_i++]);
i++;
}
}
Expand All @@ -556,14 +563,14 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
p = va_arg(ap, mrb_value*);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
*p = *sp++;
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*p = ARGV[arg_i++];
i++;
break;
}
}
if (i < argc) {
*p = to_ary(mrb, *sp++);
*p = to_ary(mrb, ARGV[arg_i++]);
i++;
}
}
Expand All @@ -575,14 +582,14 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
p = va_arg(ap, mrb_value*);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
*p = *sp++;
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*p = ARGV[arg_i++];
i++;
break;
}
}
if (i < argc) {
*p = to_hash(mrb, *sp++);
*p = to_hash(mrb, ARGV[arg_i++]);
i++;
}
}
Expand All @@ -597,15 +604,15 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
pl = va_arg(ap, mrb_int*);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*ps = NULL;
*pl = 0;
i++;
break;
}
}
if (i < argc) {
ss = to_str(mrb, *sp++);
ss = to_str(mrb, ARGV[arg_i++]);
*ps = RSTRING_PTR(ss);
*pl = RSTRING_LEN(ss);
i++;
Expand All @@ -620,14 +627,14 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
ps = va_arg(ap, const char**);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*ps = NULL;
i++; sp++;
i++; arg_i++;
break;
}
}
if (i < argc) {
ss = to_str(mrb, *sp++);
ss = to_str(mrb, ARGV[arg_i++]);
*ps = mrb_string_value_cstr(mrb, &ss);
i++;
}
Expand All @@ -644,15 +651,15 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
pl = va_arg(ap, mrb_int*);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*pb = 0;
*pl = 0;
i++; sp++;
i++; arg_i++;
break;
}
}
if (i < argc) {
aa = to_ary(mrb, *sp++);
aa = to_ary(mrb, ARGV[arg_i++]);
a = mrb_ary_ptr(aa);
*pb = a->ptr;
*pl = a->len;
Expand All @@ -666,8 +673,8 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)

p = va_arg(ap, mrb_float*);
if (i < argc) {
*p = mrb_to_flo(mrb, *sp);
sp++;
*p = mrb_to_flo(mrb, ARGV[arg_i]);
arg_i++;
i++;
}
}
Expand All @@ -678,13 +685,13 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)

p = va_arg(ap, mrb_int*);
if (i < argc) {
switch (mrb_type(*sp)) {
switch (mrb_type(ARGV[arg_i])) {
case MRB_TT_FIXNUM:
*p = mrb_fixnum(*sp);
*p = mrb_fixnum(ARGV[arg_i]);
break;
case MRB_TT_FLOAT:
{
mrb_float f = mrb_float(*sp);
mrb_float f = mrb_float(ARGV[arg_i]);

if (!FIXABLE(f)) {
mrb_raise(mrb, E_RANGE_ERROR, "float too big for int");
Expand All @@ -696,10 +703,10 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
mrb_raise(mrb, E_TYPE_ERROR, "no implicit conversion of String into Integer");
break;
default:
*p = mrb_fixnum(mrb_Integer(mrb, *sp));
*p = mrb_fixnum(mrb_Integer(mrb, ARGV[arg_i]));
break;
}
sp++;
arg_i++;
i++;
}
}
Expand All @@ -709,7 +716,7 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
mrb_bool *boolp = va_arg(ap, mrb_bool*);

if (i < argc) {
mrb_value b = *sp++;
mrb_value b = ARGV[arg_i++];
*boolp = mrb_test(b);
i++;
}
Expand All @@ -723,7 +730,7 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
if (i < argc) {
mrb_value ss;

ss = *sp++;
ss = ARGV[arg_i++];
*symp = to_sym(mrb, ss);
i++;
}
Expand All @@ -738,14 +745,14 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
type = va_arg(ap, struct mrb_data_type const*);
if (*format == '!') {
format++;
if (i < argc && mrb_nil_p(*sp)) {
if (i < argc && mrb_nil_p(ARGV[arg_i])) {
*datap = 0;
i++; sp++;
i++; arg_i++;
break;
}
}
if (i < argc) {
*datap = mrb_data_get_ptr(mrb, *sp++, type);
*datap = mrb_data_get_ptr(mrb, ARGV[arg_i++], type);
++i;
}
}
Expand Down Expand Up @@ -787,10 +794,10 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
if (argc > i) {
*pl = argc-i;
if (*pl > 0) {
*var = sp;
*var = ARGV + arg_i;
}
i = argc;
sp += *pl;
arg_i += *pl;
}
else {
*pl = 0;
Expand All @@ -803,6 +810,9 @@ mrb_get_args(mrb_state *mrb, const char *format, ...)
break;
}
}

#undef ARGV

if (!c && argc > i) {
mrb_raise(mrb, E_ARGUMENT_ERROR, "wrong number of arguments");
}
Expand Down

0 comments on commit c77123d

Please sign in to comment.