Repository navigation
fix: validate cursor offsets and pre-context chunk in GraphemeCursor public API - #183
Conversation
…public API Fuzzing (unicode-rs#170) showed that out-of-range cursor offsets or mismatched provide_context calls panic in prev_boundary/next_boundary/provide_context (slice out of bounds, unwrap on empty iterator, assert on None). Guard all three entries: return GraphemeIncomplete::InvalidOffset for out-of-range offsets, and ignore mismatched provide_context calls instead of asserting. No API change; all in-tree tests pass and the reported reproducers no longer panic.
- provide_context: chunk_start + chunk.len() could overflow with fuzzed chunk_start near usize::MAX; use saturating_add - prev_boundary: offset beyond the chunk sliced out-of-range; clamp to chunk.len() so the loop reports PrevChunk like any chunk without a preceding boundary Both were found by afl.rs fuzzing in unicode-rs#119 (arithmetic overflow and out-of-bounds slicing). Reproduced on 1.13.3; this commit makes all 5 repro cases pass.
|
Please format the code |
|
Format should be addressed now — after merging upstream |
|
Follow-up, and a correction: my message yesterday claimed fmt was addressed - that was wrong. The branch head still had one unformatted hunk (the |
|
GitHub Actions was being buggy but it finally worked. Thanks! |
2 participants
Fixes (partially) #170.
Problem
The fuzz cases in #170 (tested against 1.13.2 and current master) show three
classes of panic in
GraphemeCursor's power-user API, all reachable without-of-range arguments that the API contract expects the caller to uphold:
prev_boundary/next_boundarywith a cursor offset larger than the textlength, or a
chunk_startafter the cursor: slicingchunk[..offset - chunk_start](or[offset - chunk_start..]) goes out of bounds, or thereversed iterator is empty and
.unwrap()panics (grapheme.rs:697/787/790).provide_contextcalled without a pendingPreContextrequest (or with amismatched chunk end):
self.pre_context_offset.unwrap()panics(grapheme.rs:415).
is_boundaryat a chunk position where the requested char doesn't exist(grapheme.rs:628, same root cause as 1).
Fix
prev_boundary/next_boundary: returnErr(GraphemeIncomplete::InvalidOffset)when
self.offset > self.lenorchunk_start > self.offset.InvalidOffsetis an existing variant already used by
is_boundary, so no API change.provide_context: silently ignore a call that does not match the pendingPreContextrequest instead ofassert!-ing. The pre-context state remainspending, so a subsequent correct call still works.
No behavior change for contract-abiding callers: every in-tree test passes
unchanged, and offsets/requests that were valid before take exactly the same
code paths.
Verification
(the fourth returns
Okalready); all 4 return normally after.cargo test: 23 passed / 0 failed.Note: case 3 of the issue (
is_boundaryunwrap at grapheme.rs:628 viaprev_boundaryretry) is covered by the entry guards; a fully hardenedis_boundaryagainst all misuses may want a follow-up, but this closes everypanic case reproduced in the issue.