normalize_utf32: skip out-of-range codepoints in the compose loop (fixes #288) - #352
Open
AetherAI3 wants to merge 1 commit into
Open
Conversation
JuliaStrings#288) utf8proc_normalize_utf32's UTF8PROC_COMPOSE branch reads each buffer[rpos] and passes it to unsafe_get_property, guarded only by if (current_char < 0) continue; (a skip meant for the grapheme-break sentinel written by utf8proc_decompose_char, not general invalid input). unsafe_get_property indexes utf8proc_stage1table[uc >> 8], and utf8proc_stage1table is const utf8proc_uint16_t[4352]. So a UTF-32 codepoint >= 0x110000 in the input buffer produces uc >> 8 >= 4352 and reads past the table. Add the missing bound to the existing skip -- the same uc < 0 || uc >= 0x110000 shape the public wrapper utf8proc_get_property already uses (utf8proc.c:243) and that utf8proc_decompose_char rejects with UTF8PROC_ERROR_NOTASSIGNED (utf8proc.c:456). Invalid codepoints are dropped the same way grapheme-break sentinels already are. Reproduced under ASan against master 0075ed7 with a 3-codepoint buffer { 0x0041, 0x0301, 0x110000 } and UTF8PROC_COMPOSE: global-buffer-overflow READ of size 2 at unsafe_get_property, 0 bytes after utf8proc_stage1table (size 8704). Clean after this patch. Fixes: JuliaStrings#288 Reported-by: shuangxiangkan (github.com/shuangxiangkan) Signed-off-by: Brandon Barrante <aetherai@aethersystems.net>
stevengj
approved these changes
Sep 7, 2026
Member
|
This seems reasonable to me. (Mostly we haven't used this function directly ourselves, since we only do normalization on UTF-8 text in Julia, so by the time it is decomposed to UTF-32 it is already validated.) Can you add a test? Just something in |
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.
Fixes #288.
Small one.
utf8proc_normalize_utf32'sUTF8PROC_COMPOSEbranch reads eachbuffer[rpos]and passes it straight tounsafe_get_property, guarded only byif (current_char < 0) continue;(that skip is meant for the grapheme-break sentinel written byutf8proc_decompose_char, not for general invalid input).unsafe_get_propertyindexesutf8proc_stage1table[uc >> 8], andutf8proc_stage1tableis aconst utf8proc_uint16_t[4352]. So a UTF-32 codepoint>= 0x110000in the input buffer producesuc >> 8 >= 4352and reads past the table.The public wrapper
utf8proc_get_propertyalready handles both bounds with the patternuc < 0 || uc >= 0x110000 ? utf8proc_properties : unsafe_get_property(uc), andutf8proc_decompose_charusesif (uc < 0 || uc >= 0x110000) return UTF8PROC_ERROR_NOTASSIGNED;. The fix mirrors that same guard in the one call site that was missing it.Verification
Built master
0075ed7dwith clang +-fsanitize=address,undefined.Minimal reproducer (buffer with one out-of-range codepoint):
utf8proc.c:237:7: runtime error: index 4352 out of bounds for type 'const utf8proc_uint16_t[4352]'. ASan saysAddressSanitizer: global-buffer-overflow ... READ of size 2insideunsafe_get_property, called fromutf8proc_normalize_utf32line 669,0 bytes after global variable 'utf8proc_stage1table' ... of size 8704. Same failure class as the SEGV in Segmentation fault in utf8proc_normalize_utf32 with invalid options #288.Regression under ASan/UBSan (only the tests that don't need downloaded Unicode data — the sandbox this ran in couldn't reach unicode.org):
test/iterate— 673 tests passedtest/valid— SUCCEEDEDtest/case— 2942 tests SUCCEEDEDtest/misc— NFC round-trip SUCCEEDED, API version 17.0.0test/charwidth— 156996 chars SUCCEEDEDtest/custom— map_custom SUCCEEDEDtest/printproperty— OKHand-checked smoke: compose of
c a f e ́returnsc a f é(U+0063 U+0061 U+0066 U+00E9) unchanged.utf8proc_reencodeofhello 世界still produces the correct UTF-8 bytes.Notes on scope
The reporter mentions
utf8proc_reencodemay crash the same way.utf8proc_reencodecallsutf8proc_normalize_utf32first, so this fix covers it transitively.utf8proc_encode_charandcharbound_encode_char(called byutf8proc_reencodeafter normalize) already rejectuc >= 0x110000by returning 0.Other
unsafe_get_propertycallers were reviewed:utf8proc_decompose_char(utf8proc.c:457) — guarded at line 456 (if (uc < 0 || uc >= 0x110000) return UTF8PROC_ERROR_NOTASSIGNED;).utf8proc_decompose_customcanonical-reordering loop (utf8proc.c:609-610) — the buffer contents there come fromutf8proc_decompose_char, which only writes validated codepoints, so thoseunsafe_get_propertycalls are safe.unsafe_get_property(*starter)in the compose loop (utf8proc.c:698) —starterpoints into a buffer position we just wrote a validated codepoint into, so it inherits the guard added above.Diff
+3/-2 in one file, one function.
Base
Applied on top of master
0075ed7d0adba45682ee6bf7a83b10f8fd110163.Credit
Reported by @shuangxiangkan (#288) with an AFL++ reproducer and ASan trace. Their trace pins the crash exactly at
utf8proc.c:237— the finding did the analytical work.Note on AI assistance
I used an AI assistant to help trace the crash back to the missing bound and to draft this write-up. The code change and every claim above I verified myself against the built binary.