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

Crash when super is called with splat arguments and a block #3504

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

Comments

Projects
None yet
1 participant
@clayton-shopify
Contributor

clayton-shopify commented Mar 14, 2017

The following input demonstrates a crash:

class A
  def foo(*args)
  end
end

class B < A
  def foo(*args)
    super(*args, &:bar)
  end

  # It doesn't matter if this method is defined or not
  def bar
  end
end

B.new.foo

I haven't tracked down the root cause but I did notice that regs[1] is a MRB_TT_PROC here, but it should be an array:

mruby/src/vm.c

Line 1560 in 4b1e5d4

struct RArray *ary = mrb_ary_ptr(regs[1]);

That causes argc to be set incorrectly a couple lines later, which results in a crash here:

mruby/src/vm.c

Line 1610 in 4b1e5d4

regs[m1+o+1] = mrb_ary_new_from_values(mrb, rnum, argv+m1+o);

ASAN report:

==23760==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62f00000c430 at pc 0x00010c9969eb bp 0x7fff53605560 sp 0x7fff53604d20
READ of size 16 at 0x62f00000c430 thread T0
    #0 0x10c9969ea in __asan_memcpy (libclang_rt.asan_osx_dynamic.dylib+0x419ea)
    #1 0x10c5f5dec in array_copy array.c:71
    #2 0x10c5f5bcf in mrb_ary_new_from_values array.c:80
    #3 0x10c769bae in mrb_vm_exec (mruby+0x100178bae)
    #4 0x10c756f99 in mrb_vm_run (mruby+0x100165f99)
    #5 0x10c789639 in mrb_top_run (mruby+0x100198639)
    #6 0x10c858b45 in mrb_load_exec (mruby+0x100267b45)
    #7 0x10c859955 in mrb_load_file_cxt (mruby+0x100268955)
    #8 0x10c5f35d6 in main mruby.c:227
    #9 0x7fffaf458254 in start (libdyld.dylib+0x5254)

0x62f00000c430 is located 0 bytes to the right of 49200-byte region [0x62f000000400,0x62f00000c430)
allocated by thread T0 here:
    #0 0x10c99ff87 in wrap_realloc (libclang_rt.asan_osx_dynamic.dylib+0x4af87)
    #1 0x10c6ec675 in mrb_default_allocf (mruby+0x1000fb675)
    #2 0x10c66e228 in mrb_realloc_simple gc.c:201
    #3 0x10c66e90e in mrb_realloc gc.c:215
    #4 0x10c66f383 in mrb_malloc gc.c:236
    #5 0x10c66f41d in mrb_calloc gc.c:254
    #6 0x10c66f9c9 in add_heap gc.c:324
    #7 0x10c66f813 in mrb_gc_init gc.c:357
    #8 0x10c6ec4cc in mrb_open_core (mruby+0x1000fb4cc)
    #9 0x10c6ec7dc in mrb_open_allocf (mruby+0x1000fb7dc)
    #10 0x10c6ec7a7 in mrb_open (mruby+0x1000fb7a7)
    #11 0x10c5f24e8 in main mruby.c:171
    #12 0x7fffaf458254 in start (libdyld.dylib+0x5254)

SUMMARY: AddressSanitizer: heap-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib+0x419ea) in __asan_memcpy
Shadow bytes around the buggy address:
  0x1c5e00001830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c5e00001840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c5e00001850: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c5e00001860: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1c5e00001870: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1c5e00001880: 00 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa
  0x1c5e00001890: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e000018a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e000018b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e000018c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e000018d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==23760==ABORTING
Abort trap: 6

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

@matz matz closed this in b64f087 Mar 16, 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