Repository navigation
Commit d976e3c
authored
Fix match registers when position is larger than INT_MAX (#220)
## What
With `StringScanner.new(str, fixed_anchor: true)`, a scan position
beyond `INT_MAX` (a string longer than 2 GiB) makes
`StringScanner#getch` / `#get_byte` read out of bounds: a deterministic
`SIGSEGV` (and a bogus string returned when the address happens to be
mapped) instead of returning the byte.
## Reproducer
```ruby
require "strscan"
len = 2**31 + 64 # > INT_MAX
pos = 2**31 + 10
s = "a" * len
sc = StringScanner.new(s, fixed_anchor: true)
sc.pos = pos
p sc.getch # expected "a"; actual: SIGSEGV (exit 139)
```
On current master (`STRSCAN_VERSION 3.1.9`, built against Ruby 3.3.8):
```
control: fixed_anchor=false getch OK size=1 matched=1
trigger: calling getch with fixed_anchor=true ...
[BUG] Segmentation fault
strscan.so(extract_range) strscan.c:170
strscan.so(strscan_getch) strscan.c:1204
ruby exit code: 139
```
With the fix the same script completes normally and returns `"a"`.
## Root cause
`adjust_registers_to_matched()` stored the (64-bit) scan positions
through
`onig_region_set(region, 0, (int)p->prev, (int)p->curr)` — the `int`
parameters silently narrow the
positions. For `pos > INT_MAX` the stored `beg[0]` becomes negative;
`extract_range()` /
`extract_beg_len()` only checked the upper bound, so `S_PBEG(p) + beg_i`
is computed before the
string and `rb_str_new()` reads out of bounds.
## Changes
- store the match registers directly (`OnigPosition` values, no
narrowing).
## Tests
- Reproducer: crashes before the change / returns `"a"` after.
- `test/strscan/test_stringscanner.rb`: no new failures vs master (only
the pre-existing
standalone-harness issues).
## Notes
This was also reported through HackerOne (`#4061020`), where Hiroshi
SHIBATA closed it as
Informative (the effect is a crash that requires an
application-controlled buffer larger than
2 GiB, rather than a security vulnerability) and asked to send it here
as a pull request.
Co-authored-by: Gkasgd <7713860+Gkasgd@users.noreply.github.com>1 parent a552ea6 commit d976e3c
1 file changed
Lines changed: 7 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1183 | 1183 | | |
1184 | 1184 | | |
1185 | 1185 | | |
| 1186 | + | |
1186 | 1187 | | |
1187 | | - | |
| 1188 | + | |
| 1189 | + | |
| 1190 | + | |
| 1191 | + | |
1188 | 1192 | | |
1189 | 1193 | | |
1190 | | - | |
| 1194 | + | |
| 1195 | + | |
1191 | 1196 | | |
1192 | 1197 | | |
1193 | 1198 | | |
| |||
0 commit comments