Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signed integer overflow in mrb_str_format #4062

Closed
clayton-shopify opened this issue Jun 25, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@clayton-shopify
Copy link
Contributor

commented Jun 25, 2018

The check macro (in /mrbgems/mruby-sprintf/src/sprintf.c) contains an signed integer overflow in bsize:

#define CHECK(l) do {\
/*  int cr = ENC_CODERANGE(result);*/\
  while ((l) >= bsiz - blen) {\
    bsiz*=2;\
    if (bsiz < 0) mrb_raise(mrb, E_ARGUMENT_ERROR, "too big specifier"); \
  }\
  mrb_str_resize(mrb, result, bsiz);\
/*  ENC_CODERANGE_SET(result, cr);*/\
  buf = RSTRING_PTR(result);\
} while (0)

bsiz*=2 can become negative. However with -O2 the mrb_raise is never triggered, since bsiz is a signed integer. Signed integer overflows are undefined behaviour and thus gcc removes the check.

This results in negative integers being passed to mrb_str_resize, which will set the string length without further checks. This can potentially result in a oversized string that allows to access arbitrary memory.

In our experiments building with gcc in version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.9) produces the vulnerable binary while clang generated a safe executable.

MRB_API mrb_value
mrb_str_resize(mrb_state *mrb, mrb_value str, mrb_int len)
{
  mrb_int slen;
  struct RString *s = mrb_str_ptr(str);

  mrb_str_modify(mrb, s);
  slen = RSTR_LEN(s);
  if (len != slen) {
    if (slen < len || slen - len > 256) { //check passes if len < 0
      resize_capa(mrb, s, len);
    }
    RSTR_SET_LEN(s, len); //string len is set to a large value
    RSTR_PTR(s)[len] = '\0';   /* sentinel */ //oob access
  }
  return str;
}

Reproduce:

git clone https://github.com/mruby/mruby.git
cd mruby
git checkout 351614bbddb5f612ecf9572e975e78e058d2bf11
make 
echo "' %8646911284551352320d'%1" | ./bin/mruby
Segmentation fault (core dumped)

Backtrace in gdb:

Starting program: /home/sergej/mruby/bin/mruby < crash1

Program received signal SIGSEGV, Segmentation fault.
0x000000000041d7d7 in mrb_str_resize (mrb=mrb@entry=0x6b8010, 
    str=..., len=len@entry=-9223372036854775808)
    at /home/sergej/mruby/src/string.c:718
718	    RSTR_PTR(s)[len] = '\0';   /* sentinel */
(gdb) bt
#0  0x000000000041d7d7 in mrb_str_resize (
    mrb=mrb@entry=0x6b8010, str=..., 
    len=len@entry=-9223372036854775808)
    at /home/sergej/mruby/src/string.c:718
#1  0x000000000046615c in mrb_str_format (
    mrb=mrb@entry=0x6b8010, argc=<optimized out>, 
    argv=<optimized out>, fmt=...)
    at /home/sergej/mruby/mrbgems/mruby-sprintf/src/sprintf.c:966
#2  0x0000000000468492 in mrb_f_sprintf (mrb=0x6b8010, obj=...)
    at /home/sergej/mruby/mrbgems/mruby-sprintf/src/sprintf.c:520
#3  0x0000000000447231 in mrb_vm_exec (mrb=mrb@entry=0x6b8010, 
    proc=<optimized out>, proc@entry=0x6b8f00, 
    pc=<optimized out>) at /home/sergej/mruby/src/vm.c:1477
#4  0x0000000000449ecc in mrb_vm_run (mrb=0x6b8010, 
    proc=0x6b8f00, self=..., stack_keep=<optimized out>)
    at /home/sergej/mruby/src/vm.c:948
#5  0x000000000044bf99 in mrb_top_run (mrb=mrb@entry=0x6b8010, 
    proc=proc@entry=0x6b8f00, self=..., 
    stack_keep=stack_keep@entry=0)
    at /home/sergej/mruby/src/vm.c:3010
#6  0x0000000000441021 in mrb_load_exec (mrb=mrb@entry=0x6b8010, 
    p=<optimized out>, c=c@entry=0x711850)
    at /home/sergej/mruby/mrbgems/mruby-compiler/core/parse.y:5835
#7  0x0000000000442e85 in mrb_load_file_cxt (
    mrb=mrb@entry=0x6b8010, f=0x7ffff7ac88e0 <_IO_2_1_stdin_>, 
    c=c@entry=0x711850)
    at /home/sergej/mruby/mrbgems/mruby-compiler/core/parse.y:5844
#8  0x0000000000402dc7 in main (argc=<optimized out>, 
    argv=<optimized out>)
    at /home/sergej/mruby/mrbgems/mruby-bin-mruby/tools/mruby/mruby.c:279

Credits:

This issue was reported by Sergej Schumilo, Daniel Teuchert and Cornelius Aschermann (https://hackerone.com/schumilo).

Impact

This results in negative integers being passed to mrb_str_resize, which will set the string length without further checks. This can potentially result in a oversized string that allows to access arbitrary memory.

@matz matz closed this in 695f29c Jun 27, 2018

matz added a commit that referenced this issue Jun 28, 2018

@clayton-shopify

This comment has been minimized.

Copy link
Contributor Author

commented Jul 19, 2018

The extra check added in 695f29c prevents the crash, but undefined behaviour is still invoked. This can be seen using UndefinedBehaviorSanitizer in clang:

$ echo "' %8646911284551352320d'%1" | bin/mruby
/Users/clayton/git/mruby/mrbgems/mruby-sprintf/src/sprintf.c:966:11: runtime error: signed integer overflow: 8646911284551352320 * 2 cannot be represented in type 'long long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /Users/clayton/git/mruby/mrbgems/mruby-sprintf/src/sprintf.c:966:11 in
trace (most recent call last):
	[0] -:1
-:1: too big specifier (ArgumentError)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.