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 Outdated
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants