Skip to content

Commit

Permalink
Resize ary when Array#sort! block modifies embedded ary
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fresh-eggs authored and jhawthorn committed May 1, 2024
1 parent 66554a5 commit 95ae419
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
3 changes: 3 additions & 0 deletions array.c
Original file line number Diff line number Diff line change
Expand Up @@ -3387,6 +3387,9 @@ rb_ary_sort_bang(VALUE ary)
rb_ary_unshare(ary);
FL_SET_EMBED(ary);
}
if (ARY_EMBED_LEN(tmp) > ARY_CAPA(ary)) {
ary_resize_capa(ary, ARY_EMBED_LEN(tmp));
}
ary_memcpy(ary, 0, ARY_EMBED_LEN(tmp), ARY_EMBED_PTR(tmp));
ARY_SET_LEN(ary, ARY_EMBED_LEN(tmp));
}
Expand Down
9 changes: 9 additions & 0 deletions test/ruby/test_array.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3555,6 +3555,15 @@ def test_big_array_literal_with_kwsplat
assert_equal(10000, eval(lit).size)
end

def test_array_safely_modified_by_sort_block
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)
assert_equal((1..67).to_a.reverse, var_0)
end

private
def need_continuation
unless respond_to?(:callcc, true)
Expand Down

0 comments on commit 95ae419

Please sign in to comment.