Repository navigation
Conversation
…FFFF Since the refactor in owasp-modsecurity#3284 (2.9.9) the output pointer always advanced by six bytes after sprintf("%%u%04x"), so a code point that needs five hex digits (anything above U+FFFF) lost its last digit to the next write or to the terminating NUL: U+1F600 came out as "%u1f60". Advance by the number of bytes sprintf actually wrote. While there, fix two adjacent checks in the same function: bytes_left was never decremented, so the "enough bytes left" guards compared the total length instead of the remaining length and a truncated sequence at the end of the value was examined past input_len; and an 0xF4 lead byte with a second byte >= 0x90 (U+110000 and above, outside Unicode) was accepted as a valid four-byte sequence. Add four-byte, above-U+10FFFF and truncated-sequence cases to the utf8toUnicode unit tests, which had no four-byte input at all. Fixes owasp-modsecurity#3684 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Cq9GqRpB4wzTFnEUvQaUJg
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This branch has not been deployed
2 participants
Proposed changes
Fixes #3684. Related to #1211 (the 4-vs-6 digit output ambiguity, which this PR does not change).
Since the refactor in #3284 (first released in 2.9.9)
utf8_unicode_inplace_ex()always advanced the output pointer by six bytes aftersprintf("%%u%04x"), so any code point above U+FFFF (five hex digits) lost its last digit to the next write or to the terminating NUL: U+1F600 came out as%u1f60. The pointer now advances by whatsprintf()actually wrote.Two adjacent checks in the same function are fixed as well:
bytes_leftwas never decremented, so the "enough bytes left" guards compared against the total input length instead of the remaining length, and a truncated sequence at the end of the value was examined pastinput_len(bounded in practice by the NUL that terminates variable values, but wrong).0xF4lead byte with a second byte>= 0x90(U+110000 and above, outside Unicode) was accepted as a valid four-byte sequence; it is now passed through like any other invalid sequence.Output is otherwise unchanged, including the pre-existing handling of a NUL byte (
%u00xxfrom #3284) and of the BMP.Tests
tests/tfn/utf8toUnicode.thad no four-byte input at all. Four cases are added: supplementary-plane characters, a four-byte character followed by ASCII, a sequence above U+10FFFF, and a truncated sequence at the end of the input. Against the unmodifiedv2/mastersource the first three fail (Passed: 8; Failed: 3); with this patch all 11 pass andmake testis green (macOS, Apache 2.4 from Homebrew, PCRE2 10.49). The function was also exercised standalone under AddressSanitizer with the same inputs.🤖 Generated with Claude Code