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 use-after-free in gc_mark_children #3679

Closed
clayton-shopify opened this Issue May 30, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@clayton-shopify
Contributor

clayton-shopify commented May 30, 2017

The following input demonstrates a crash:

a = [0]
31.times { a[0,0] = a }

I reproduced this on an Ubuntu 14.04 virtual machine with 1 GB of RAM.

The crash began at 4566c80, which added a free after a failed realloc. Maybe the caller of mrb_default_allocf isn't expecting the buffer to be freed in that case?

Backtrace:

(gdb) bt
#0  0x000000000040368c in gc_mark_children (mrb=0x6b0010, gc=0x6b00d0, obj=0x6b1d60) at /home/clayton/git/mruby/src/gc.c:678
#1  0x0000000000403e10 in gc_gray_mark (mrb=0x6b0010, gc=0x6b00d0, obj=0x6b1d60) at /home/clayton/git/mruby/src/gc.c:892
#2  0x00000000004040a9 in incremental_marking_phase (mrb=0x6b0010, gc=0x6b00d0, limit=18446744073709551615)
    at /home/clayton/git/mruby/src/gc.c:987
#3  0x00000000004044eb in incremental_gc (mrb=0x6b0010, gc=0x6b00d0, limit=18446744073709551615) at /home/clayton/git/mruby/src/gc.c:1091
#4  0x00000000004045a0 in incremental_gc_until (mrb=0x6b0010, gc=0x6b00d0, to_state=MRB_GC_STATE_ROOT)
    at /home/clayton/git/mruby/src/gc.c:1116
#5  0x00000000004046b0 in clear_all_old (mrb=0x6b0010, gc=0x6b00d0) at /home/clayton/git/mruby/src/gc.c:1142
#6  0x0000000000404961 in mrb_full_gc (mrb=0x6b0010) at /home/clayton/git/mruby/src/gc.c:1208
#7  0x0000000000402623 in mrb_realloc_simple (mrb=0x6b0010, p=0x7fff978fc010, len=1073741824) at /home/clayton/git/mruby/src/gc.c:205
#8  0x0000000000402677 in mrb_realloc (mrb=0x6b0010, p=0x7fff978fc010, len=1073741824) at /home/clayton/git/mruby/src/gc.c:217
#9  0x000000000041ff03 in ary_expand_capa (mrb=0x6b0010, a=0x6b1d60, len=67108864) at /home/clayton/git/mruby/src/array.c:193
#10 0x00000000004213d8 in mrb_ary_splice (mrb=0x6b0010, ary=..., head=0, len=0, rpl=...) at /home/clayton/git/mruby/src/array.c:653
#11 0x0000000000421973 in mrb_ary_aset (mrb=0x6b0010, self=...) at /home/clayton/git/mruby/src/array.c:833
#12 0x000000000040c1f4 in mrb_vm_exec (mrb=0x6b0010, proc=0x6b1d30, pc=0x720254) at /home/clayton/git/mruby/src/vm.c:1335
#13 0x000000000040a313 in mrb_vm_run (mrb=0x6b0010, proc=0x6b1d90, self=..., stack_keep=0) at /home/clayton/git/mruby/src/vm.c:868
#14 0x0000000000412e20 in mrb_top_run (mrb=0x6b0010, proc=0x6b1d90, self=..., stack_keep=0) at /home/clayton/git/mruby/src/vm.c:2770
#15 0x000000000044a58a in mrb_load_exec (mrb=0x6b0010, p=0x70e2e0, c=0x70cf30)
    at /home/clayton/git/mruby/mrbgems/mruby-compiler/core/parse.y:5780
#16 0x000000000044a620 in mrb_load_file_cxt (mrb=0x6b0010, f=0x70df20, c=0x70cf30)
    at /home/clayton/git/mruby/mrbgems/mruby-compiler/core/parse.y:5789
#17 0x0000000000402415 in main (argc=2, argv=0x7fffffffe478) at /home/clayton/git/mruby/mrbgems/mruby-bin-mruby/tools/mruby/mruby.c:227

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

@Asmod4n

This comment has been minimized.

Show comment
Hide comment
@Asmod4n

Asmod4n May 30, 2017

Contributor

This is what i meant in #3658 (comment)
It shouldn't free the memory if realloc fails.
mirb should instead be fixed in that regard and not mruby.
e.g. the realloc in https://github.com/mruby/mruby/blob/master/mrbgems/mruby-bin-mirb/tools/mirb/mirb.c#L279 should be inside a MRB_TRY/MRB_CATCH block where the original args->argv should be restored when a exception is thrown.

Contributor

Asmod4n commented May 30, 2017

This is what i meant in #3658 (comment)
It shouldn't free the memory if realloc fails.
mirb should instead be fixed in that regard and not mruby.
e.g. the realloc in https://github.com/mruby/mruby/blob/master/mrbgems/mruby-bin-mirb/tools/mirb/mirb.c#L279 should be inside a MRB_TRY/MRB_CATCH block where the original args->argv should be restored when a exception is thrown.

@matz matz closed this in 7dbbd77 May 31, 2017

matz added a commit that referenced this issue May 31, 2017

Prevent splicing big recursive arrrays; ref #3679
We know this is not perfect, but this change makes hack like #3679
bit harder. Harmless for useful cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment