mruby-regexp: answer a byte search before the subject with a miss - #7153

Regexp.__byte_search is the byte-offset search that the mrblib loops of gsub, sub, split, scan and byteindex drive themselves, and it takes the position it is handed without asking anything about it. That is what those loops want: they enter at zero or at an offset a match answered with, and they already work in byte space. A position past the end needs no question either, because the engine reads it as a miss.

A position before the subject is read instead. It reaches the engine as RSTRING_PTR(str) + pos, and the walk starts from there:

$ mruby -e 'Regexp.__byte_search(/x/, "a" * 100, -1)'
AddressSanitizer: heap-buffer-overflow, READ of size 101
    #0 memchr
    #1 literal_exec mrbgems/mruby-regexp/src/re_exec.c:857
    #2 mrb_re_exec mrbgems/mruby-regexp/src/re_exec.c:889
    #3 exec_match mrbgems/mruby-regexp/src/regexp.c:381
    #4 regexp_s_byte_search mrbgems/mruby-regexp/src/regexp.c:505

A large enough one segfaults rather than reading a neighbour.

Where a negative position can come from

No mrblib caller passes one. gsub, sub, split and scan start at zero and resume from a match offset; byteindex reads its argument against the byte length and answers both ends itself before it searches. The way in is a direct call to the internal class method, which Ruby code can make because it is defined on Regexp.

So what this adds is a backstop rather than a gate, of the kind check_regexp_arg() already is for the pattern in the same function: every mrblib caller passes a Regexp, and the entry point asks anyway.

I checked the gem's other internal entry points for the same shape. __search and __search_p normalize through re_char_to_byte(), which answers a miss for anything out of range. __scan, __sub_str, __gsub_str, __byte_begin and __byte_end answer or raise for every position I gave them. __byte_search is the one that reads.

The answer it gives

The miss the far end already gives, clearing the match globals the same way, so the two ends of the range come out alike. A search that publishes nothing clears nothing at either end, which is the contract #7149 gave that argument. Nothing that reaches this today changes.

IndexError was the alternative. It would say more, but it would also make the two ends of the range disagree, and it would be the only raise on a path whose whole contract is that the caller has settled the position. The miss keeps the entry point as quiet as it is.

Tests

The new assertions read $~ back after each call, because the answer and the clearing are one act here, and they cover the non-publishing form at both ends. They are in test/regexp.rb rather than test/string_index.rb, since this is the entry point rather than a String method.

The current tree cannot pin the old behavior first: it is a read out of bounds, so a test for it either crashes mrbtest or reads a neighbouring byte, depending on the build. The test and the change are one commit for that reason.

Verification

rake -m test on the default build and on an address,undefined sanitizer build of full-core, green on both.

Reverting only the change in regexp.c and keeping the test turns the sanitizer build red at the same address, so the assertions are load bearing.

Related

#7152 adds Regexp.__check_byte_pos, an entry point of the same kind, and it wants the same backstop. That is independent of this and stays in that PR.

Summary by CodeRabbit

• Bug Fixes
  • Regular expression searches with negative or excessively large byte offsets now return nil instead of producing an incorrect match.
  • Published searches clear the match state for invalid offsets, while non-publishing searches preserve the existing match state.