[Bug #22467]
If we perform IO read without the GVL during GC compaction and the string
we are writing to is embedded, then it can get mprotect'd by the read
barrier in the GC, which can cause the IO read operation to fail.
rb_str_subpat_set validated a negative capture index by negating it,
which overflows for INT_MIN and bypasses the range check. The
out-of-bounds read of the match registers then crashed the process.
Rewrite the check without negation.
Reported by: loki (https://hackerone.com/realnits)
Co-Authored-By: Claude Mythos 5.1 <noreply@anthropic.com>
Some example results in the docs no longer match what Ruby prints:
* Array#flatten and Array#flatten!: Set#inspect now shows Set[6, 7]
* Module: Mod.constants also includes Math::DomainError
* Module#const_get and Module#const_set: Floats are shown with full
precision
* Complex#imag: sin(PI/4) is 0.7071067811865475
* String#<<: missing # in #<Encoding:UTF-8>
* Enumerable#inject: typo #=>>
long is 32 bits on mswin while pointers are 64 bits, so String, Array and MatchData could not reach 2GiB there. config.h now sets SIZEOF_RB_LEN_T to 8 on mswin64, and value.h then defines rb_len_t as int64_t. mingw still uses long.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Without capacity:, rb_enc_raw_set doesn't resize the buffer for a wider
terminator, so the terminator could overflow an embedded string.
rb_enc_associate resizes it.
With capacity: and no encoding:, the buffer was sized for a 1-byte
terminator even when orig or the receiver has a wide encoding.
Follow up 2d08b5f42f [Bug #22423]
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Negating RB_LEN_MIN overflows, so the compiler may assume `-beg < 0` never holds and drop the check. clang-cl does so, and `str[LONG_MIN, 1]` returns a substring instead of nil.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A range of single character Strings told that it includes its end
even when it is empty, since the end was not compared with the begin.
("z".."a").to_a #=> []
("z".."a").include?("a") #=> true
These still held String and Array lengths and indices in long, or cast them to long. rb_len_t is still long everywhere, so the generated code does not change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
These range checks guard values that are now rb_len_t but still compared SIZEOF_LONG, so they would be compiled out once rb_len_t gets wider than long on LLP64. They compile the same while rb_len_t is long. ibf_dump_write takes the size as size_t so that its check sees the full value.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RString capa becomes pointer-sized everywhere once rb_len_t widens, so fake strings can keep their precomputed hash on LLP64 as well.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Once rb_len_t widens this changes the `long *` output parameters of rb_range_beg_len(), rb_str_subpos(), rb_arithmetic_sequence_beg_len_step(), ruby_enc_find_basename() and ruby_enc_find_extname(), so extensions passing `long *` to them will need to be updated. The extension guides list these signatures verbatim and follow along.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit renames rb_str_tmp_frozen_no_embed_acquire to rb_str_no_gvl_safe_acquire
and makes it part of the public C API.
There is currently no way for C extensions that use no GVL operations to
safely read from a string because it has no way of guaranteeing that the
string is not embedded. Embedded strings are not safe for no GVL operations
because GC compaction can run, which will mprotect the memory region and
will cause any read to SEGV.
See https://github.com/ruby/ruby/pull/9817 for when rb_str_tmp_frozen_no_embed_acquire
was introduced.
The buffer is copied over based on the new string's termlen
so we can just set the new encoding raw instead of recalculating
based on the old encoding's termlen.
[Bug #22382]
rb_str_tmp_frozen_no_embed_acquire is not Ractor safe for the non-embedded
case because it modifies the orig string. This can cause a race condition
if multiple Ractors are executing the same code path for the same string.
Since commit 9144c914a1, strings that are
pinned are not re-embedded, so we can safely return the original string
if it is frozen and not embedded.
The following script reproduces the crash:
# frozen_string_literal: true
require "tmpdir"
dir = Dir.mktmpdir
TRIES = 300
1.upto(TRIES) do |try|
$stderr.puts "try #{try} of #{TRIES}"
str = Ractor.make_shareable(Random.bytes(512 * 1024))
# Control: one prior write from a single Ractor prevents the crash.
File.binwrite(File.join(dir, "first"), str) if ENV["WRITE_FIRST"]
go = Process.clock_gettime(Process::CLOCK_MONOTONIC) + 0.02
12.times.map do |n|
Ractor.new(str, File.join(dir, "w#{n}"), go) do |s, path, at|
Thread.pass until Process.clock_gettime(Process::CLOCK_MONOTONIC) >= at # start together
File.binwrite(path, s)
end
end.each(&:join)
GC.start
end
puts "finished #{TRIES} tries without crashing"
capa does not include the terminator, so when a buffer moves
between strings with different terminator lengths the capacity has to
be converted or we will try to free the buffer with the wrong size:
[BUG] buffer 0x00005030039ff768 freed with old_size=18, but was allocated with size=17
[BUG] buffer 0x00005631adc48f48 freed with old_size=8268, but was allocated with size=8269
Three related fixes:
* `rb_str_tmp_frozen_release`: convert tmp's capacity to orig's
terminator length when handing the buffer back. This is the inverse
of a calculation added to `rb_str_tmp_frozen_no_embed_acquire` in
91619f0230.
* `heap_str_make_shared`: since bdf3032e35
the root created by `rb_str_tmp_frozen_acquire` does not copy the
encoding of the string it takes the buffer from, so the two can have
different terminator lengths and the root's capacity has to be
adjusted as well, otherwise the root frees fewer bytes than were
allocated once its only child stops sharing the buffer. termlen is
passed in because the caller assigns the encoding of the root after
this function returns.
* `rb_str_tmp_frozen_no_embed_acquire`: unset `STR_BORROWED` on the
root. `STR_SET_SHARED` sets `STR_BORROWED` on roots with
`klass == 0` to mark the ones that may have other children, but this
root only ever has orig as its child, so unset it like
`heap_str_make_shared` does. Any string that starts sharing the
root later will set the flag again. Without this
`rb_str_tmp_frozen_release` never gives the buffer back, so strings
written by `io_fwrite` stay shared and are copied on their next
modification.
The release and make_shared adjustments go together
as they were previously canceling each other out.
45a2c95d0f mistakenly rewrote
FL_TEST_RAW(orig, STR_SHARED) && !FL_TEST_RAW(orig, STR_TMPLOCK|RUBY_FL_FREEZE)
as
FL_TEST_RAW(orig, STR_SHARED | STR_TMPLOCK) == STR_TMPLOCK
requiring orig to not be shared, the opposite of the original
condition. orig always shares the buffer with tmp here, so the buffer
was never given back and the string stayed shared until its next
modification copied it. The code then read aux.shared from strings
that are not shared, where the union holds aux.capa.
This should be backported to 4.0
The conversion of the pattern to string can cause the string to be modified.
This can cause a buffer overflow since it uses the original length of the
string. For example, the following script reports a buffer overflow with
ASAN:
s = "héllo" * 1000
obj = Object.new
obj.define_singleton_method(:to_str) do
s.replace("hé")
"l"
end
p s.rpartition(obj)
If the source string is modified when converting offset to an integer,
then there could be an out-of-bounds access because the length of the
string is captured before to_int is called. For example, the following
script triggers an ASAN error:
s = "héllo" * 1_000_000
obj = Object.new
obj.define_singleton_method(:to_int) do
s.replace("é" * 1000)
450000
end
p s.byteindex("l", obj)
String#tr can crash if keys are removed from the translation hash since
we stack allocate a buffer pairs. If entries are deleted during runtime,
then we won't fill the pairs buffer which can crash because it will be
reading uninitialized values out of pairs.
The following script demonstrates the crash:
h = {}
obj = Object.new
obj.define_singleton_method(:to_str) do
h.clear
"a"
end
h[obj] = "x"
("b".."z").each { |c| h[c] = (c.ord + 1).chr }
puts "ab".tr!(h)
If the array object passed into String#% is modified during conversion
(such as via to_s, to_i, to_f, etc.) then it can be a use-after-free if
the buffer of the array is freed.
This commit changes it to detect if the buffer has been changed and raises
an error when that happens.
The following script reproduces the crash:
count = 1_000
obj = Object.new
ary = [obj, *Array.new(count - 1) { "x" }]
obj.define_singleton_method(:to_s) do
ary.replace([])
"X"
end
str = "%s" * count
puts (str % ary).length
The separator can be modified in the block yielded by String#each_line.
When that happens, the pointer can change which can cause an use-after-free.
For example, the following script crashes:
sep = "x" * 1_000_000
s = "a#{sep}b#{sep}c"
s.each_line(sep) { sep.clear }
The selector conversion may run aribtrary Ruby code and modify the source
string. This can cause a use-after-free if it resizes the source string.
For example, the following script causes corruption:
s = "-" * 1000 + "abc" + "-" * 1000
obj = Object.new
obj.define_singleton_method(:to_str) do
s.clear
"-"
end
p s.strip(obj)
GCC was emitting a warning, because it couldn't understand that
`search->needles_count` was assumed not to be 0:
```
string.c:9673:36: warning: ‘matches’ may be used uninitialized [-Wmaybe-uninitialized]
9673 | const int bitmap = _mm_movemask_epi8(matches[0]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
string.c: In function ‘tr_trans_pairs’:
string.c:9664:25: note: ‘matches’ declared here
9664 | __m128i matches[TR_TRANS_PAIRS_SIMD_MAX_NEEDLES];
|
```
Instead of adding some useless initialization (41c9fdd65c)
we can try copying the loop upper bound into a `const` so that the compiler
can understand it's never 0;
The following script causes a buffer overflow and returns corrupted strings
because it's reading past the end of the string buffer:
s = "x" + "l" * 3999
obj = Object.new
obj.define_singleton_method(:to_int) do
s.clear
0
end
p s.slice!(/l+$/, obj)
Outputs corrupted string that looks like:
"\xEA\x94\xEA\xCE`\u0000\u0000`\x88n\xEFC
string.c:9673:36: warning: ‘matches’ may be used uninitialized [-Wmaybe-uninitialized]
9673 | const int bitmap = _mm_movemask_epi8(matches[0]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
string.c: In function ‘tr_trans_pairs’:
string.c:9664:25: note: ‘matches’ declared here
9664 | __m128i matches[TR_TRANS_PAIRS_SIMD_MAX_NEEDLES];
|
The call to rb_to_int may run aribtrary Ruby code and thus cause String#setbyte
to read past the end of the string. For example, the following script crashes:
s = "a" * 10_000_000
obj = Object.new
obj.define_singleton_method(:to_int) do
s.clear
s.concat("x" * 50)
0
end
s.setbyte(9_999_996, obj)
* `tr_trans_pair` is now responsible for consuming the matches,
akin to the ERB change in 4ca48a192c.
This makes it impossible to shift by 64 or more, hence removes a
branch in `next_match`. Also ensure `s` and `matches_bitmap` members
stays in sync.
* Add missing `needles_count` check in `tr_trans_pairs_search_sse2`.
Reported-By: Ashley Allen <a.allen3@herts.ac.uk>
Reported-By: Asten Rooky
`tr_trans_pairs_search` expect the pointer to be incremented by
one only. Therefore, when matching a multibyte character, `search.s`
must be incremented by 1, and not `clen`.
Co-Authored-By: Federico Carrocera <fedecarrocera@gmail.com>
String#chomp steps back by the encoding's minimum character width before
testing for a newline, which reads before the first byte of the receiver
when it holds less than one whole character.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The same character count clamp reaches the backward scan in str_rindex() and
the pos == 0 shortcut in rb_str_rindex(), where a pattern with wider
characters than the receiver is still compared past its last byte.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Peter Zhu