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_each_objects #3616

Closed
clayton-shopify opened this Issue Apr 18, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@clayton-shopify
Contributor

clayton-shopify commented Apr 18, 2017

The following input demonstrates a crash:

ObjectSpace.each_object { GC.start }

ASAN report:

==28981==ERROR: AddressSanitizer: heap-use-after-free on address 0x62f00000e410 at pc 0x000100f949bc bp 0x7fff5ece51e0 sp 0x7fff5ece51d8
READ of size 8 at 0x62f00000e410 thread T0
    #0 0x100f949bb in gc_each_objects gc.c:1504
    #1 0x100f948ca in mrb_objspace_each_objects gc.c:1511
    #2 0x1011161e6 in os_each_object mruby_objectspace.c:170
    #3 0x101082618 in mrb_vm_exec (mruby:x86_64+0x100171618)
    #4 0x10107779f in mrb_vm_run (mruby:x86_64+0x10016679f)
    #5 0x1010aa8b9 in mrb_top_run (mruby:x86_64+0x1001998b9)
    #6 0x10117bba5 in mrb_load_exec (mruby:x86_64+0x10026aba5)
    #7 0x10117c4f5 in mrb_load_file_cxt (mruby:x86_64+0x10026b4f5)
    #8 0x100f139e6 in main mruby.c:227
    #9 0x7fffbbbba234 in start (libdyld.dylib:x86_64+0x5234)

0x62f00000e410 is located 16 bytes inside of 49200-byte region [0x62f00000e400,0x62f00001a430)
freed by thread T0 here:
    #0 0x101328356 in wrap_free (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x56356)
    #1 0x10100c44b in mrb_default_allocf (mruby:x86_64+0x1000fb44b)
    #2 0x100f8e9b9 in mrb_free gc.c:269
    #3 0x100f98e2f in incremental_sweep_phase gc.c:1063
    #4 0x100f9735c in incremental_gc gc.c:1104
    #5 0x100f93326 in incremental_gc_until gc.c:1120
    #6 0x100f93727 in clear_all_old gc.c:1146
    #7 0x100f8d8f1 in mrb_full_gc gc.c:1212
    #8 0x100f94cd3 in gc_start gc.c:1324
    #9 0x101082618 in mrb_vm_exec (mruby:x86_64+0x100171618)
    #10 0x10107779f in mrb_vm_run (mruby:x86_64+0x10016679f)
    #11 0x10107042e in mrb_run (mruby:x86_64+0x10015f42e)
    #12 0x101076436 in mrb_yield_with_class (mruby:x86_64+0x100165436)
    #13 0x101077118 in mrb_yield (mruby:x86_64+0x100166118)
    #14 0x101117410 in os_each_object_cb mruby_objectspace.c:139
    #15 0x100f94978 in gc_each_objects gc.c:1501
    #16 0x100f948ca in mrb_objspace_each_objects gc.c:1511
    #17 0x1011161e6 in os_each_object mruby_objectspace.c:170
    #18 0x101082618 in mrb_vm_exec (mruby:x86_64+0x100171618)
    #19 0x10107779f in mrb_vm_run (mruby:x86_64+0x10016679f)
    #20 0x1010aa8b9 in mrb_top_run (mruby:x86_64+0x1001998b9)
    #21 0x10117bba5 in mrb_load_exec (mruby:x86_64+0x10026aba5)
    #22 0x10117c4f5 in mrb_load_file_cxt (mruby:x86_64+0x10026b4f5)
    #23 0x100f139e6 in main mruby.c:227
    #24 0x7fffbbbba234 in start (libdyld.dylib:x86_64+0x5234)

previously allocated by thread T0 here:
    #0 0x101328520 in wrap_realloc (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x56520)
    #1 0x10100c465 in mrb_default_allocf (mruby:x86_64+0x1000fb465)
    #2 0x100f8d6d8 in mrb_realloc_simple gc.c:202
    #3 0x100f8ddbe in mrb_realloc gc.c:216
    #4 0x100f8e843 in mrb_malloc gc.c:237
    #5 0x100f8e8dd in mrb_calloc gc.c:255
    #6 0x100f8ee89 in add_heap gc.c:325
    #7 0x100f92017 in mrb_obj_alloc gc.c:511
    #8 0x100ffb683 in mrb_proc_new (mruby:x86_64+0x1000ea683)
    #9 0x100fcb8d4 in mrb_load_irep_cxt (mruby:x86_64+0x1000ba8d4)
    #10 0x100fcca1f in mrb_load_irep (mruby:x86_64+0x1000bba1f)
    #11 0x101105d2f in GENERATED_TMP_mrb_mruby_proc_ext_gem_init (mruby:x86_64+0x1001f4d2f)
    #12 0x10119b5d7 in mrb_init_mrbgems (mruby:x86_64+0x10028a5d7)
    #13 0x10100c5f1 in mrb_open_allocf (mruby:x86_64+0x1000fb5f1)
    #14 0x10100c597 in mrb_open (mruby:x86_64+0x1000fb597)
    #15 0x100f128f8 in main mruby.c:171
    #16 0x7fffbbbba234 in start (libdyld.dylib:x86_64+0x5234)

SUMMARY: AddressSanitizer: heap-use-after-free gc.c:1504 in gc_each_objects
Shadow bytes around the buggy address:
  0x1c5e00001c30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e00001c40: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e00001c50: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e00001c60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x1c5e00001c70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x1c5e00001c80: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c5e00001c90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c5e00001ca0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c5e00001cb0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c5e00001cc0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x1c5e00001cd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==28981==ABORTING
Abort trap: 6

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

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Apr 19, 2017

Member

Crash seems to be fixed by 77c2aa7

Member

matz commented Apr 19, 2017

Crash seems to be fixed by 77c2aa7

@ssarongg

This comment has been minimized.

Show comment
Hide comment
@ssarongg

ssarongg Apr 19, 2017

It's memory corruption issue so you should compile mruby with ASAN.
I confirmed that the PoC works on 6a0b68f

ssarongg commented Apr 19, 2017

It's memory corruption issue so you should compile mruby with ASAN.
I confirmed that the PoC works on 6a0b68f

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Apr 19, 2017

Contributor

@matz I can also confirm that the crash is still present in 6a0b68f. Valgrind also reports the problem:

==58930== Invalid read of size 8
==58930==    at 0x4051DA: gc_each_objects (gc.c:1504)
==58930==    by 0x40521D: mrb_objspace_each_objects (gc.c:1511)
==58930==    by 0x465EAE: os_each_object (mruby_objectspace.c:170)
==58930==    by 0x40BE35: mrb_vm_exec (vm.c:1304)
==58930==    by 0x40A0A5: mrb_vm_run (vm.c:854)
==58930==    by 0x4128F0: mrb_top_run (vm.c:2705)
==58930==    by 0x44B257: mrb_load_exec (parse.y:5780)
==58930==    by 0x44B2ED: mrb_load_file_cxt (parse.y:5789)
==58930==    by 0x402414: main (mruby.c:227)
==58930==  Address 0x558a2a0 is 16 bytes inside a block of size 49,200 free'd
==58930==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58930==    by 0x406F9A: mrb_default_allocf (state.c:56)
==58930==    by 0x40281E: mrb_free (gc.c:269)
==58930==    by 0x404400: incremental_sweep_phase (gc.c:1063)
==58930==    by 0x4045BE: incremental_gc (gc.c:1104)
==58930==    by 0x404622: incremental_gc_until (gc.c:1120)
==58930==    by 0x404732: clear_all_old (gc.c:1146)
==58930==    by 0x4049C5: mrb_full_gc (gc.c:1212)
==58930==    by 0x404E02: gc_start (gc.c:1324)
==58930==    by 0x40BE35: mrb_vm_exec (vm.c:1304)
==58930==    by 0x40A0A5: mrb_vm_run (vm.c:854)
==58930==    by 0x412883: mrb_run (vm.c:2694)

I think the problem is that gc_each_objects does not take into account that callback can change the state of the heap pages.

Contributor

clayton-shopify commented Apr 19, 2017

@matz I can also confirm that the crash is still present in 6a0b68f. Valgrind also reports the problem:

==58930== Invalid read of size 8
==58930==    at 0x4051DA: gc_each_objects (gc.c:1504)
==58930==    by 0x40521D: mrb_objspace_each_objects (gc.c:1511)
==58930==    by 0x465EAE: os_each_object (mruby_objectspace.c:170)
==58930==    by 0x40BE35: mrb_vm_exec (vm.c:1304)
==58930==    by 0x40A0A5: mrb_vm_run (vm.c:854)
==58930==    by 0x4128F0: mrb_top_run (vm.c:2705)
==58930==    by 0x44B257: mrb_load_exec (parse.y:5780)
==58930==    by 0x44B2ED: mrb_load_file_cxt (parse.y:5789)
==58930==    by 0x402414: main (mruby.c:227)
==58930==  Address 0x558a2a0 is 16 bytes inside a block of size 49,200 free'd
==58930==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==58930==    by 0x406F9A: mrb_default_allocf (state.c:56)
==58930==    by 0x40281E: mrb_free (gc.c:269)
==58930==    by 0x404400: incremental_sweep_phase (gc.c:1063)
==58930==    by 0x4045BE: incremental_gc (gc.c:1104)
==58930==    by 0x404622: incremental_gc_until (gc.c:1120)
==58930==    by 0x404732: clear_all_old (gc.c:1146)
==58930==    by 0x4049C5: mrb_full_gc (gc.c:1212)
==58930==    by 0x404E02: gc_start (gc.c:1324)
==58930==    by 0x40BE35: mrb_vm_exec (vm.c:1304)
==58930==    by 0x40A0A5: mrb_vm_run (vm.c:854)
==58930==    by 0x412883: mrb_run (vm.c:2694)

I think the problem is that gc_each_objects does not take into account that callback can change the state of the heap pages.

@matz matz closed this in d2cad9a Apr 20, 2017

@clayton-shopify

This comment has been minimized.

Show comment
Hide comment
@clayton-shopify

clayton-shopify Apr 20, 2017

Contributor

@matz It's still possible to cause a crash with:

ObjectSpace.each_object { GC.enable ; GC.start }
Contributor

clayton-shopify commented Apr 20, 2017

@matz It's still possible to cause a crash with:

ObjectSpace.each_object { GC.enable ; GC.start }
@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Apr 20, 2017

Member

Ah, OK.

Member

matz commented Apr 20, 2017

Ah, OK.

@matz matz reopened this Apr 20, 2017

@Asmod4n

This comment has been minimized.

Show comment
Hide comment
@Asmod4n

Asmod4n Apr 23, 2017

Contributor

Oh, thanks for this fix, i was wondering why i couldn't access symbols in ObjectSpace.each_object from C in a gem finalizer method.

ASAN showed that error too, you might be able to spot these kind of errors with adding

conf.cc.flags << '-fsanitize=address'
conf.linker.flags << '-fsanitize=address'

to https://github.com/mruby/mruby/blob/master/travis_config.rb where ASAN is possible.

Contributor

Asmod4n commented Apr 23, 2017

Oh, thanks for this fix, i was wondering why i couldn't access symbols in ObjectSpace.each_object from C in a gem finalizer method.

ASAN showed that error too, you might be able to spot these kind of errors with adding

conf.cc.flags << '-fsanitize=address'
conf.linker.flags << '-fsanitize=address'

to https://github.com/mruby/mruby/blob/master/travis_config.rb where ASAN is possible.

@matz matz closed this in 058da1f Apr 25, 2017

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

@matz

This comment has been minimized.

Show comment
Hide comment
@matz

matz Apr 25, 2017

Member

I hope 058da1f fixed the issue.

Member

matz commented Apr 25, 2017

I hope 058da1f fixed the issue.

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