Skip to content

Conversation

@fresh-eggs
Copy link
Contributor

(note: It was decided we should handle this in the public issue tracker in security ticket #2327648)

In cases where rb_ary_sort_bang is called with a block and tmp is an embedded array, we need to account for the block potentially impacting the capacity of ary.

Reproduction script for x86 targets:

var_0 = (1..70).to_a var_0.sort! do |var_0_block_129, var_1_block_129| var_0.pop var_1_block_129 <=> var_0_block_129 end.shift(3)

Reproduction script for ARM targets:

10.times do var_0 = (1..70).to_a var_0.sort! do |var_0_block_129, var_1_block_129| var_0.pop var_1_block_129 <=> var_0_block_129 end.shift(3) end

The above example can put the array into a corrupted state (ary after block has len=0 and capa=14) :

================== ary =================== ary: BD99908 is_embedded?: 0 is_shared?: 0 heap.len: 0 heap.capa: 14 heap.shared_root: 14 ================== tmp =================== ary: BD1EB18 is_embedded?: 1 is_shared?: 0 embed_len: 70 embed_capa: 78 heap.len: 141 heap.capa: 139 heap.shared_root: 139 

This results in a heap buffer overflow and possible segfault:

==19964==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60b0000034f0 at pc 0x00010c35ee6c bp 0x0003070fb290 sp 0x0003070faa50 WRITE of size 560 at 0x60b0000034f0 thread T0 #0 0x10c35ee6b in wrap_memcpy+0x2ab (libclang_rt.asan_osx_dynamic.dylib:x86_64+0x18e6b) #1 0x100e0b085 in ruby_nonempty_memcpy memory.h:671 #2 0x100e0e43e in ary_memcpy0 array.c:335 #3 0x100e0cb00 in ary_memcpy array.c:352 #4 0x100e1426c in rb_ary_sort_bang array.c:3519 [ ... ] 

Was able to reproduce on the following builds:

  • ruby 3.4.0dev (2024-01-17T14:48:46Z ef4a08e) [x86_64-linux]
  • ruby 3.3.0 (2024-01-05 revision 634d4e2) [x86_64-darwin23]
  • ruby 3.3.0 (2023-12-25 revision 5124f9a) [arm64-darwin23]

Could not reproduce on the following builds:

  • ruby 3.2.3 (2024-01-18 revision 52bb2ac) [x86_64-linux]
  • ruby 3.1.4p249 (2024-01-11 revision 2b60834) [x86_64-linux]

This commit adds a conditional to determine when the capacity of ary has been modified by the provided block. If this is the case, ensure that the capacity of ary is adjusted to handle at minimum the len of tmp.

test-all passes locally:

Finished tests in 70.194526s, 369.6727 tests/s, 89373.2939 assertions/s. 25949 tests, 6273516 assertions, 0 failures, 0 errors, 292 skips 
In cases where `rb_ary_sort_bang` is called with a block and tmp is an embedded array, we need to account for the block potentially impacting the capacity of ary. ex: ``` var_0 = (1..70).to_a var_0.sort! do |var_0_block_129, var_1_block_129| var_0.pop var_1_block_129 <=> var_0_block_129 end.shift(3) ``` The above example can put the array into a corrupted state resulting in a heap buffer overflow and possible segfault: ``` ERROR: AddressSanitizer: heap-buffer-overflow on address [...] WRITE of size 560 at 0x60b0000034f0 thread T0 [...] ``` This commit adds a conditional to determine when the capacity of ary has been modified by the provided block. If this is the case, ensure that the capacity of ary is adjusted to handle at minimum the len of tmp.
@nobu
Copy link
Member

nobu commented Apr 13, 2024

@nobu nobu merged commit c479492 into ruby:master Apr 13, 2024
@fresh-eggs
Copy link
Contributor Author

Done!
https://bugs.ruby-lang.org/issues/20427

Thanks for your help, I included the patch in the issue in git am format. It should apply to latest ruby_3_3.

Let me know if you need anything else.

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

Labels

None yet

2 participants