ICU-23334 Fix null-deref and stale UErrorCode in break_iterator_fuzzer#3887
Open
OwenSanzas wants to merge 1 commit intounicode-org:mainfrom
Open
Conversation
Two issues in the break_iterator_fuzzer fuzz harness: 1. bi->setText(fuzzstr, status) on line 57 dereferences `bi` without checking for nullptr. If utext_openUChars or createXxxInstance fails (setting status to error), `bi` is nullptr and the subsequent call causes a null pointer dereference (SEGV). Add null/error checks after utext_openUChars and after createXxxInstance. 2. The second round of createXxxInstance calls (lines 70-86) reuses `status` without resetting it to U_ZERO_ERROR. If the first phase left status in an error state, all second-phase ICU calls silently return without executing. Add `status = U_ZERO_ERROR` before the second phase.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix two issues in
icu4c/source/test/fuzzer/break_iterator_fuzzer.cpp:1. Null pointer dereference (line 57)
bi->setText(fuzzstr, status)dereferencesbiwithout checking for nullptr. Ifutext_openUCharsorcreateXxxInstancefails (e.g., due to OOM settingstatusto error),biis nullptr and the call causes a SEGV.Confirmed with AddressSanitizer:
Fix: Add null/error checks after
utext_openUCharsand aftercreateXxxInstance, before callingbi->setText().2. UErrorCode not reset before reuse (line 70)
The second round of
createXxxInstancecalls (lines 70-86) reusesstatuswithout resetting it toU_ZERO_ERROR. ICU's API protocol requiresUErrorCodeto beU_ZERO_ERRORbefore each call — if a previous call set it to error, subsequent ICU APIs silently return without executing. This makes the second phase of the harness effectively dead code whenever the first phase encounters any error.Fix: Add
status = U_ZERO_ERRORbefore the second phase.Before/After Comparison (60-second fuzzing, AddressSanitizer)
Coverage is identical. No crashes in either version during 60-second run. The fixed version runs slightly faster.
Changes
Three minimal additions (no behavioral changes to the happy path):
utext_openUCharsreturn value before usebifor nullptr before callingsetTextstatusbefore secondcreateXxxInstanceround