Skip to content

Incorrect utf8toUnicode transformation for 00xx - #3284

Merged
marcstern merged 2 commits into
owasp-modsecurity:v2/masterfrom
marcstern:v2/pr/utf8toUnicodeVsMultibyte
Nov 12, 2024
Merged

marcstern merged 2 commits into
owasp-modsecurity:v2/masterfrom
marcstern:v2/pr/utf8toUnicodeVsMultibyte

Conversation

@marcstern

Copy link
Copy Markdown

Fix issue and restructure handling
#3129

Perform all the checks and calculate the multibute length, then write the bytes at a single place (the correct way).

Fix issue and restructure handling
@marcstern marcstern added the 2.x Related to ModSecurity version 2.x label Oct 22, 2024
Comment thread apache2/msc_util.c Resolved
@airween

airween commented Oct 31, 2024

Copy link
Copy Markdown
Member

Hi @marcstern,

I added one comment above, and would like to ask if we modify a function that used by a transformation (utf8toUnicode) then probably we should add a test case to our CI. What do you think?

@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2024

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarCloud

@airween

airween commented Nov 12, 2024

Copy link
Copy Markdown
Member

@marcstern do you think we can add a test case to check this transformation's behavior? Or it's not necessary?

@marcstern
marcstern merged commit fa621f8 into owasp-modsecurity:v2/master Nov 12, 2024
@marcstern

Copy link
Copy Markdown
Author

We should ideally add test for this, indeed

airween pushed a commit that referenced this pull request Oct 15, 2026
…FFFF

Since the refactor in #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 #3684

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Cq9GqRpB4wzTFnEUvQaUJg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.x Related to ModSecurity version 2.x

2 participants