Skip to content

[Backport 3.3] [Bug #20688] Fix use-after-free for WeakMap and WeakKeyMap #11439

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

Merged
merged 7 commits into from
Aug 22, 2024

Conversation

peterzhu2118
Copy link
Member

We cannot free the weakmap_entry before the ST_DELETE because it could hash the key which would read the weakmap_entry and would cause a use-after-free. Instead, we store the entry and free it on the next iteration.

For example, the following script triggers a use-after-free in Valgrind:

weakmap = ObjectSpace::WeakMap.new
10_000.times { weakmap[Object.new] = Object.new }
==25795== Invalid read of size 8
==25795==    at 0x462297: wmap_cmp (weakmap.c:165)
==25795==    by 0x3A2B1C: find_table_bin_ind (st.c:930)
==25795==    by 0x3A5EAA: st_general_foreach (st.c:1599)
==25795==    by 0x3A5EAA: rb_st_foreach (st.c:1640)
==25795==    by 0x25C991: gc_mark_children (default.c:4870)
==25795==    by 0x25C991: gc_marks_wb_unprotected_objects_plane (default.c:5565)
==25795==    by 0x25C991: rgengc_rememberset_mark_plane (default.c:5557)
==25795==    by 0x25C991: rgengc_rememberset_mark (default.c:6233)
==25795==    by 0x25C991: gc_marks_start (default.c:6057)
==25795==    by 0x25C991: gc_marks (default.c:6077)
==25795==    by 0x25C991: gc_start (default.c:6723)
==25795==    by 0x260F96: heap_prepare (default.c:2282)
==25795==    by 0x260F96: heap_next_free_page (default.c:2489)
==25795==    by 0x260F96: newobj_cache_miss (default.c:2598)
==25795==    by 0x26197F: newobj_alloc (default.c:2622)
==25795==    by 0x26197F: rb_gc_impl_new_obj (default.c:2701)
==25795==    by 0x26197F: newobj_of (gc.c:890)
==25795==    by 0x26197F: rb_wb_protected_newobj_of (gc.c:917)
==25795==    by 0x2DEA88: rb_class_allocate_instance (object.c:131)
==25795==    by 0x2E3B18: class_call_alloc_func (object.c:2141)
==25795==    by 0x2E3B18: rb_class_alloc (object.c:2113)
==25795==    by 0x2E3B18: rb_class_new_instance_pass_kw (object.c:2172)
==25795==    by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786)
==25795==    by 0x44B08D: vm_sendish (vm_insnhelper.c:5953)
==25795==    by 0x44B08D: vm_exec_core (insns.def:898)
==25795==    by 0x43A7A4: rb_vm_exec (vm.c:2564)
==25795==    by 0x234914: rb_ec_exec_node (eval.c:281)
==25795==  Address 0x21603710 is 0 bytes inside a block of size 16 free'd
==25795==    at 0x4849B2C: free (vg_replace_malloc.c:989)
==25795==    by 0x249651: rb_gc_impl_free (default.c:8527)
==25795==    by 0x249651: rb_gc_impl_free (default.c:8508)
==25795==    by 0x249651: ruby_sized_xfree.constprop.0 (gc.c:4178)
==25795==    by 0x4626EC: ruby_sized_xfree_inlined (gc.h:277)
==25795==    by 0x4626EC: wmap_free_entry (weakmap.c:45)
==25795==    by 0x4626EC: wmap_mark_weak_table_i (weakmap.c:61)
==25795==    by 0x3A5CEF: apply_functor (st.c:1633)
==25795==    by 0x3A5CEF: st_general_foreach (st.c:1543)
==25795==    by 0x3A5CEF: rb_st_foreach (st.c:1640)
==25795==    by 0x25C991: gc_mark_children (default.c:4870)
==25795==    by 0x25C991: gc_marks_wb_unprotected_objects_plane (default.c:5565)
==25795==    by 0x25C991: rgengc_rememberset_mark_plane (default.c:5557)
==25795==    by 0x25C991: rgengc_rememberset_mark (default.c:6233)
==25795==    by 0x25C991: gc_marks_start (default.c:6057)
==25795==    by 0x25C991: gc_marks (default.c:6077)
==25795==    by 0x25C991: gc_start (default.c:6723)
==25795==    by 0x260F96: heap_prepare (default.c:2282)
==25795==    by 0x260F96: heap_next_free_page (default.c:2489)
==25795==    by 0x260F96: newobj_cache_miss (default.c:2598)
==25795==    by 0x26197F: newobj_alloc (default.c:2622)
==25795==    by 0x26197F: rb_gc_impl_new_obj (default.c:2701)
==25795==    by 0x26197F: newobj_of (gc.c:890)
==25795==    by 0x26197F: rb_wb_protected_newobj_of (gc.c:917)
==25795==    by 0x2DEA88: rb_class_allocate_instance (object.c:131)
==25795==    by 0x2E3B18: class_call_alloc_func (object.c:2141)
==25795==    by 0x2E3B18: rb_class_alloc (object.c:2113)
==25795==    by 0x2E3B18: rb_class_new_instance_pass_kw (object.c:2172)
==25795==    by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786)
==25795==    by 0x44B08D: vm_sendish (vm_insnhelper.c:5953)
==25795==    by 0x44B08D: vm_exec_core (insns.def:898)
==25795==    by 0x43A7A4: rb_vm_exec (vm.c:2564)
==25795==  Block was alloc'd at
==25795==    at 0x484680F: malloc (vg_replace_malloc.c:446)
==25795==    by 0x25CE9E: rb_gc_impl_malloc (default.c:8542)
==25795==    by 0x462A39: wmap_aset_replace (weakmap.c:423)
==25795==    by 0x3A5542: rb_st_update (st.c:1487)
==25795==    by 0x462B8E: wmap_aset (weakmap.c:452)
==25795==    by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786)
==25795==    by 0x44B08D: vm_sendish (vm_insnhelper.c:5953)
==25795==    by 0x44B08D: vm_exec_core (insns.def:898)
==25795==    by 0x43A7A4: rb_vm_exec (vm.c:2564)
==25795==    by 0x234914: rb_ec_exec_node (eval.c:281)
==25795==    by 0x2369B8: ruby_run_node (eval.c:319)
==25795==    by 0x15D675: rb_main (main.c:43)
==25795==    by 0x15D675: main (main.c:62)

[Bug #20688]

We cannot free the weakmap_entry before the ST_DELETE because it could
hash the key which would read the weakmap_entry and would cause a
use-after-free. Instead, we store the entry and free it on the next
iteration.

For example, the following script triggers a use-after-free in Valgrind:

    weakmap = ObjectSpace::WeakMap.new
    10_000.times { weakmap[Object.new] = Object.new }

    ==25795== Invalid read of size 8
    ==25795==    at 0x462297: wmap_cmp (weakmap.c:165)
    ==25795==    by 0x3A2B1C: find_table_bin_ind (st.c:930)
    ==25795==    by 0x3A5EAA: st_general_foreach (st.c:1599)
    ==25795==    by 0x3A5EAA: rb_st_foreach (st.c:1640)
    ==25795==    by 0x25C991: gc_mark_children (default.c:4870)
    ==25795==    by 0x25C991: gc_marks_wb_unprotected_objects_plane (default.c:5565)
    ==25795==    by 0x25C991: rgengc_rememberset_mark_plane (default.c:5557)
    ==25795==    by 0x25C991: rgengc_rememberset_mark (default.c:6233)
    ==25795==    by 0x25C991: gc_marks_start (default.c:6057)
    ==25795==    by 0x25C991: gc_marks (default.c:6077)
    ==25795==    by 0x25C991: gc_start (default.c:6723)
    ==25795==    by 0x260F96: heap_prepare (default.c:2282)
    ==25795==    by 0x260F96: heap_next_free_page (default.c:2489)
    ==25795==    by 0x260F96: newobj_cache_miss (default.c:2598)
    ==25795==    by 0x26197F: newobj_alloc (default.c:2622)
    ==25795==    by 0x26197F: rb_gc_impl_new_obj (default.c:2701)
    ==25795==    by 0x26197F: newobj_of (gc.c:890)
    ==25795==    by 0x26197F: rb_wb_protected_newobj_of (gc.c:917)
    ==25795==    by 0x2DEA88: rb_class_allocate_instance (object.c:131)
    ==25795==    by 0x2E3B18: class_call_alloc_func (object.c:2141)
    ==25795==    by 0x2E3B18: rb_class_alloc (object.c:2113)
    ==25795==    by 0x2E3B18: rb_class_new_instance_pass_kw (object.c:2172)
    ==25795==    by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786)
    ==25795==    by 0x44B08D: vm_sendish (vm_insnhelper.c:5953)
    ==25795==    by 0x44B08D: vm_exec_core (insns.def:898)
    ==25795==    by 0x43A7A4: rb_vm_exec (vm.c:2564)
    ==25795==    by 0x234914: rb_ec_exec_node (eval.c:281)
    ==25795==  Address 0x21603710 is 0 bytes inside a block of size 16 free'd
    ==25795==    at 0x4849B2C: free (vg_replace_malloc.c:989)
    ==25795==    by 0x249651: rb_gc_impl_free (default.c:8527)
    ==25795==    by 0x249651: rb_gc_impl_free (default.c:8508)
    ==25795==    by 0x249651: ruby_sized_xfree.constprop.0 (gc.c:4178)
    ==25795==    by 0x4626EC: ruby_sized_xfree_inlined (gc.h:277)
    ==25795==    by 0x4626EC: wmap_free_entry (weakmap.c:45)
    ==25795==    by 0x4626EC: wmap_mark_weak_table_i (weakmap.c:61)
    ==25795==    by 0x3A5CEF: apply_functor (st.c:1633)
    ==25795==    by 0x3A5CEF: st_general_foreach (st.c:1543)
    ==25795==    by 0x3A5CEF: rb_st_foreach (st.c:1640)
    ==25795==    by 0x25C991: gc_mark_children (default.c:4870)
    ==25795==    by 0x25C991: gc_marks_wb_unprotected_objects_plane (default.c:5565)
    ==25795==    by 0x25C991: rgengc_rememberset_mark_plane (default.c:5557)
    ==25795==    by 0x25C991: rgengc_rememberset_mark (default.c:6233)
    ==25795==    by 0x25C991: gc_marks_start (default.c:6057)
    ==25795==    by 0x25C991: gc_marks (default.c:6077)
    ==25795==    by 0x25C991: gc_start (default.c:6723)
    ==25795==    by 0x260F96: heap_prepare (default.c:2282)
    ==25795==    by 0x260F96: heap_next_free_page (default.c:2489)
    ==25795==    by 0x260F96: newobj_cache_miss (default.c:2598)
    ==25795==    by 0x26197F: newobj_alloc (default.c:2622)
    ==25795==    by 0x26197F: rb_gc_impl_new_obj (default.c:2701)
    ==25795==    by 0x26197F: newobj_of (gc.c:890)
    ==25795==    by 0x26197F: rb_wb_protected_newobj_of (gc.c:917)
    ==25795==    by 0x2DEA88: rb_class_allocate_instance (object.c:131)
    ==25795==    by 0x2E3B18: class_call_alloc_func (object.c:2141)
    ==25795==    by 0x2E3B18: rb_class_alloc (object.c:2113)
    ==25795==    by 0x2E3B18: rb_class_new_instance_pass_kw (object.c:2172)
    ==25795==    by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786)
    ==25795==    by 0x44B08D: vm_sendish (vm_insnhelper.c:5953)
    ==25795==    by 0x44B08D: vm_exec_core (insns.def:898)
    ==25795==    by 0x43A7A4: rb_vm_exec (vm.c:2564)
    ==25795==  Block was alloc'd at
    ==25795==    at 0x484680F: malloc (vg_replace_malloc.c:446)
    ==25795==    by 0x25CE9E: rb_gc_impl_malloc (default.c:8542)
    ==25795==    by 0x462A39: wmap_aset_replace (weakmap.c:423)
    ==25795==    by 0x3A5542: rb_st_update (st.c:1487)
    ==25795==    by 0x462B8E: wmap_aset (weakmap.c:452)
    ==25795==    by 0x429DDC: vm_call_cfunc_with_frame_ (vm_insnhelper.c:3786)
    ==25795==    by 0x44B08D: vm_sendish (vm_insnhelper.c:5953)
    ==25795==    by 0x44B08D: vm_exec_core (insns.def:898)
    ==25795==    by 0x43A7A4: rb_vm_exec (vm.c:2564)
    ==25795==    by 0x234914: rb_ec_exec_node (eval.c:281)
    ==25795==    by 0x2369B8: ruby_run_node (eval.c:319)
    ==25795==    by 0x15D675: rb_main (main.c:43)
    ==25795==    by 0x15D675: main (main.c:62)
[Bug #20688]

We cannot free the key before the ST_DELETE because it could hash the
key which would read the key and would cause a use-after-free. Instead,
we store the key and free it on the next iteration.
@k0kubun k0kubun merged commit 8657de7 into ruby:ruby_3_3 Aug 22, 2024
96 of 97 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants