Bug #18138
closedArray#slice! invalid memory access
Description
As of 4f24255, the array.c functions rb_ary_slice_bang / ary_slice_bang_by_rb_ary_splice allow a length to be passed to rb_ary_new4 that is too long and which leads to an invalid memory access.
This bug is present in Ruby v3_0_0, v3_0_1, and v3_0_2.
Reproduction¶
This ruby snippet will reproduce valgrind memory warnings:
(1..5000).to_a.slice!(-2, 5000)
The valgrind memory warnings on master
look like:
==228628== Invalid read of size 8
==228628== at 0x48428C0: memmove (vg_replace_strmem.c:1271)
==228628== by 0x356542: ary_memcpy (array.c:316)
==228628== by 0x356542: rb_ary_tmp_new_from_values (array.c:785)
==228628== by 0x356542: rb_ary_new_from_values (array.c:795)
==228628== by 0x356542: ary_slice_bang_by_rb_ary_splice (array.c:4106)
==228628== by 0x35E1DB: rb_ary_slice_bang (array.c:4186)
Fix¶
The fix I'm suggesting is in pull request https://github.com/ruby/ruby/pull/4787
Saving you a click:
diff --git a/array.c b/array.c
index bd323cd..edac216 100644
--- a/array.c
+++ b/array.c
@@ -4096,7 +4096,7 @@ ary_slice_bang_by_rb_ary_splice(VALUE ary, long pos, long len)
else if (orig_len < pos) {
return Qnil;
}
- else if (orig_len < pos + len) {
+ if (orig_len < pos + len) {
len = orig_len - pos;
}
if (len == 0) {
Updated by nobu (Nobuyoshi Nakada) over 2 years ago
- Status changed from Open to Closed
- Backport changed from 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN to 2.6: DONTNEED, 2.7: DONTNEED, 3.0: REQUIRED
Updated by nagachika (Tomoyuki Chikanaga) over 2 years ago
- Backport changed from 2.6: DONTNEED, 2.7: DONTNEED, 3.0: REQUIRED to 2.6: DONTNEED, 2.7: DONTNEED, 3.0: DONE
ruby_3_0 8899fa0b3d41fd27dd1a2c6f75106cb78ff27236 merged revision(s) d43279edacd09edf3a43e02d62f5be475e7c3bcb,5dc36ddcd00fc556c04c15ce9770c5a84d7d43dc,523bf31564f160f899f8cf9f73540d6a6f687f17.
Updated by mdalessio (Mike Dalessio) over 2 years ago
A colleague asked if this bug should have a CVE number, given that any application that might take offsets as untrusted input could be tricked into:
- accessing unrelated memory - CWE-125 (https://cwe.mitre.org/data/definitions/125.html)
- crashing (Denial of Service) - CWE-248 (https://cwe.mitre.org/data/definitions/248.html)
I'm not a security professional, but it does seem to me as though a CVE should be discussed.