Repository navigation
pcre2posix: do not truncate the subject length to int in regexec - #1022
Conversation
|
Thanks! Both points taken. Added the rm_so and rm_eo validation on the REG_STARTEND path and the INT_MAX clamp on the output side so oversized ovector values become minus one instead of wrapping. Suite green, pushed. |
| pmatch[i].rm_so = (ovector[i * 2] == PCRE2_UNSET || ovector[i * 2] + so > INT_MAX) ? -1 : (int)(ovector[i * 2] + so); | ||
| pmatch[i].rm_eo = (ovector[i * 2 + 1] == PCRE2_UNSET || ovector[i * 2 + 1] + so > INT_MAX) ? -1 : (int)(ovector[i * 2 + 1] + so); |
There was a problem hiding this comment.
Ooh, sorry! I'm going to be annoying, and request one more change actually.
If one of the two offsets is out-of-bounds for INT_MAX, then both fields should be set to -1.
(I also like to cast (PCRE2_SIZE)INT_MAX so there isn't a type mismatch on the comparison.)
| pmatch[i].rm_so = (ovector[i * 2] == PCRE2_UNSET || ovector[i * 2] + so > INT_MAX) ? -1 : (int)(ovector[i * 2] + so); | |
| pmatch[i].rm_eo = (ovector[i * 2 + 1] == PCRE2_UNSET || ovector[i * 2 + 1] + so > INT_MAX) ? -1 : (int)(ovector[i * 2 + 1] + so); | |
| if (ovector[i * 2] == PCRE2_UNSET || ovector[i * 2] + so > (PCRE2_SIZE)INT_MAX || ovector[i * 2 + 1] == PCRE2_UNSET || ovector[i * 2 + 1] + so > (PCRE2_SIZE)INT_MAX) | |
| { | |
| pmatch[i].rm_so = -1; | |
| pmatch[i].rm_eo = -1; | |
| } | |
| else | |
| { | |
| pmatch[i].rm_so = (int)(ovector[i * 2] + so); | |
| pmatch[i].rm_eo = (int)(ovector[i * 2 + 1] + so); | |
| } |
There was a problem hiding this comment.
No worries at all, that is a better shape. Pushed your version, both fields now go to minus one together and the comparisons use the explicit cast. Suite green.
|
I opened #1025 to make my TODO lists a bit more public. |
Problem
pcre2_regexec()truncates the subject length toint(src/pcre2posix.c):For a NUL-terminated subject longer than
INT_MAXbytes (the non-REG_STARTENDpath),(int)strlenwraps negative andeo - sosign-extends to a hugePCRE2_SIZE, sopcre2_matchscans far past the subject end.Reproduced on an ASan+UBSan build with a >2GiB subject and a non-matching pattern:
runtime error: addition of unsigned offset ... overflowedinpcre2_match.c, thenSEGV ... READ in _platform_memchr <- pcre2_match_8 <- pcre2_regexec pcre2posix.c:419.Fix
Hold
so/eoasPCRE2_SIZEand drop the cast — two lines. Theregmatch_toutput fields keep their POSIX-mandatedint(offsets beyond 2GiB cannot be represented there anyway); what is fixed is the length passed to the matcher, so over-long subjects now return a cleanREG_NOMATCHinstead of crashing.Verification
Full ctest suite passes; the ASan repro above returns
REG_NOMATCHafter the fix. Companion to #1020 (same int-width class, different file and API, more severe effect).