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

Null pointer dereference in OP_ENTER #3590

Closed
clayton-shopify opened this Issue Apr 4, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@clayton-shopify
Contributor

clayton-shopify commented Apr 4, 2017

The following input demonstrates a crash:

class A
  def foo
  end
end

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

B.new.foo

ASAN report:

==68149==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000028 (pc 0x0001007a4d01 bp 0x7fff5f5d0170 sp 0x7fff5f5c8660 T0)
==68149==The signal is caused by a READ memory access.
==68149==Hint: address points to the zero page.
    #0 0x1007a4d00 in mrb_vm_exec vm.c:1573
    #1 0x100792f39 in mrb_vm_run vm.c:824
    #2 0x1007c5849 in mrb_top_run vm.c:2630
    #3 0x100896845 in mrb_load_exec parse.y:5762
    #4 0x100897655 in mrb_load_file_cxt parse.y:5771
    #5 0x10062fe66 in main mruby.c:227
    #6 0x7fffbbbba234 in start (libdyld.dylib:x86_64+0x5234)

==68149==Register values:
rax = 0x0000000000000028  rbx = 0xf2f20000f2f2f200  rcx = 0x0000000000000028  rdx = 0x0000100000000005
rdi = 0x0000100000000000  rsi = 0x0000100000000000  rbp = 0x00007fff5f5d0170  rsp = 0x00007fff5f5c8660
 r8 = 0x0000100000000000   r9 = 0x0000100000000000  r10 = 0x0000000000000080  r11 = 0xffffe1d0a0a4de00
r12 = 0xf2f20000f1f1f1f1  r13 = 0x00001fffebeb9e84  r14 = 0xf2f20000f2f2f2f2  r15 = 0xf2f2f2f2f2040000
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV vm.c:1573 in mrb_vm_exec
==68149==ABORTING
Abort trap: 6

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

@matz matz closed this in d9fb8b6 Apr 5, 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