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

Segfault due to integer overflow in mrb_str_format #3439

Closed
clayton-shopify opened this Issue Feb 8, 2017 · 1 comment

Comments

Projects
None yet
1 participant
@clayton-shopify
Contributor

clayton-shopify commented Feb 8, 2017

The following input demonstrates a crash:

s = "hello"
sprintf("abcdefghijklmnopqrstuvwxyz % 2147483640s", s)

I think the crash happens because of an integer overflow in CHECK:

while (blen + (l) >= bsiz) {\
bsiz*=2;\
}\

This issue was reported by https://hackerone.com/aerodudrizzt

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Feb 8, 2017

Contributor

I forgot to copy in the rest of aerodudrizzt's report. Here it is:


Here is the core dump:

Core was generated by `ext/mruby_engine/mruby/build/host/bin/mirb'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0xb768bd7a in ?? ()
(gdb) bt
#0  0xb768bd7a in ?? ()
#1  0x00000006 in ?? ()
#2  0x0813d000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
(gdb) info reg
eax            0x20202020	538976288
ecx            0x1fff62c7	536830663
edx            0x1	1
ebx            0x800	2048
esp            0xbfe037c4	0xbfe037c4
ebp            0x78	0x78
esi            0x81d7dc8	136150472
edi            0x8216000	136404992
eip            0xb768bd7a	0xb768bd7a
eflags         0x10202	[ IF RF ]
cs             0x73	115
ss             0x7b	123
ds             0x7b	123
es             0x7b	123
fs             0x0	0
gs             0x33	51

Tech Details

The sprintf.c file in the mrb_str_format() function is responsible for the building of the formatted string. When using a "width" value for anything (string, char, number, etc...) there is an integer-overflow in the CHECK() macro that is responsible for assuring the string's capacity will be enough for the operation:

#define CHECK(l) do { \
  while (blen + (l) >= bsiz) {\
    bsiz*=2;\
  }\
  mrb_str_resize(mrb, result, bsiz);\
  buf += RSTRING_PTR(result);\
} while(0)

Proposed Fix

Since bsiz is always bigger-or-equal to blen the right check should be:

#define CHECK(l) do { \
  while ((l) >= bsiz - blen) {\
    bsiz*=2;\
  }\
  mrb_str_resize(mrb, result, bsiz);\
  buf += RSTRING_PTR(result);\
} while(0)

This way no arithmetic operation will done over the "width" parameter which can be up to MAX_INT in size, causing many possible integer-overflow.

Contributor

clayton-shopify commented Feb 8, 2017

I forgot to copy in the rest of aerodudrizzt's report. Here it is:


Here is the core dump:

Core was generated by `ext/mruby_engine/mruby/build/host/bin/mirb'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0xb768bd7a in ?? ()
(gdb) bt
#0  0xb768bd7a in ?? ()
#1  0x00000006 in ?? ()
#2  0x0813d000 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)
(gdb) info reg
eax            0x20202020	538976288
ecx            0x1fff62c7	536830663
edx            0x1	1
ebx            0x800	2048
esp            0xbfe037c4	0xbfe037c4
ebp            0x78	0x78
esi            0x81d7dc8	136150472
edi            0x8216000	136404992
eip            0xb768bd7a	0xb768bd7a
eflags         0x10202	[ IF RF ]
cs             0x73	115
ss             0x7b	123
ds             0x7b	123
es             0x7b	123
fs             0x0	0
gs             0x33	51

Tech Details

The sprintf.c file in the mrb_str_format() function is responsible for the building of the formatted string. When using a "width" value for anything (string, char, number, etc...) there is an integer-overflow in the CHECK() macro that is responsible for assuring the string's capacity will be enough for the operation:

#define CHECK(l) do { \
  while (blen + (l) >= bsiz) {\
    bsiz*=2;\
  }\
  mrb_str_resize(mrb, result, bsiz);\
  buf += RSTRING_PTR(result);\
} while(0)

Proposed Fix

Since bsiz is always bigger-or-equal to blen the right check should be:

#define CHECK(l) do { \
  while ((l) >= bsiz - blen) {\
    bsiz*=2;\
  }\
  mrb_str_resize(mrb, result, bsiz);\
  buf += RSTRING_PTR(result);\
} while(0)

This way no arithmetic operation will done over the "width" parameter which can be up to MAX_INT in size, causing many possible integer-overflow.

@matz matz closed this in ff03a9a Feb 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment