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

Heap buffer overflow in mrb_str_to_dbl #3551

Closed
clayton-shopify opened this Issue Mar 27, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@clayton-shopify
Contributor

clayton-shopify commented Mar 27, 2017

The following input demonstrates a crash:

%W[0 0 000000000000000000000000].map(&:to_f) while true

ASAN report:

==6422==ERROR: AddressSanitizer: SEGV on unknown address 0x603000031e00 (pc 0x000106affe28 bp 0x7fff59207eb0 sp 0x7fff59207c60 T0)
    #0 0x106affe27 in mrb_str_to_dbl (mruby:x86_64+0x100111e27)
    #1 0x106b1b527 in mrb_str_to_f (mruby:x86_64+0x10012d527)
    #2 0x106b4bee9 in mrb_exec_irep (mruby:x86_64+0x10015dee9)
    #3 0x106b4d605 in mrb_f_send (mruby:x86_64+0x10015f605)
    #4 0x106b5d2d0 in mrb_vm_exec (mruby:x86_64+0x10016f2d0)
    #5 0x106b52959 in mrb_vm_run (mruby:x86_64+0x100164959)
    #6 0x106b84c39 in mrb_top_run (mruby:x86_64+0x100196c39)
    #7 0x106c53a25 in mrb_load_exec (mruby:x86_64+0x100265a25)
    #8 0x106c54835 in mrb_load_file_cxt (mruby:x86_64+0x100266835)
    #9 0x1069f03a6 in main mruby.c:227
    #10 0x7fffbbbba234 in start (libdyld.dylib:x86_64+0x5234)

This issue was reported by Dinko Galetic and Denis Kasak (https://hackerone.com/dgaletic).

I did a bit of investigation and it appears the crash happens when a garbage collection occurs during an OP_ARRAY:

irep 0x60c0000133c0 nregs=4 nlocals=1 pools=2 syms=2 reps=0
file: 215879.rb
    1 000 OP_JMP	007
    1 001 OP_STRING	R1	L(0)	; "0"
    1 002 OP_STRING	R2	L(0)	; "0"
    1 003 OP_STRING	R3	L(1)	; "000000000000000000000000"
    1 004 OP_ARRAY	R1	R1	3    <---------- CRASH HERE
    1 005 OP_LOADSYM	R2	:to_f
    1 006 OP_SENDB	R1	:map	0
    1 007 OP_LOADT	R1
    1 008 OP_JMPIF	R1	001
    1 009 OP_LOADNIL	R1
    1 010 OP_STOP

During the garbage collection's mark_context_stack phase, nregs is 3 (even though I believe it should be 4 to match the irep) and so the string allocated in instruction 003 (just above the OP_ARRAY) gets garbage collected.

Maybe the context isn't restored correctly after the OP_SENDB returns?

@matz matz closed this in 071164b Apr 1, 2017

matz added a commit that referenced this issue Apr 2, 2017

christopheraue added a commit to christopheraue/mruby that referenced this issue Aug 12, 2017

Removed unneeded ci->nregs checks in OP_SEND and OP_SUPER
Because of #3504 `ci->nregs = bidx+1` was introduced in b64f087.
This led to the follow up error #3551 whose fix introduced the `if (bidx >= ci->nregs)`
check in 071164b and the `stack_extend(mrb, ci->nregs)`
in 93d8029.

Then, the code causing #3504 reappeared again in #3590. The fix for it moved the code
dealing with the block in OP_SUPER from below the `cipush`  to above the `cipush`
in d9fb8b6. The `if (bidx >= ci->nregs) { ... }` from
then on works with the original callinfo and not the pushed one. `ci->nregs` needed to
be modified for the pushed one because it is initialized to 0. But for the original ci
it is propertly set and a check is not needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment