Skip to content

reject overlong and surrogate UTF-8 in ippValidateAttribute - #180

Merged
michaelrsweet merged 2 commits into
OpenPrinting:masterfrom
tanjiroK-coder:ipp-validate-utf8-overlong
Oct 16, 2026
Merged

michaelrsweet merged 2 commits into
OpenPrinting:masterfrom
tanjiroK-coder:ipp-validate-utf8-overlong

Conversation

@tanjiroK-coder

Copy link
Copy Markdown
Contributor

ippValidateAttribute checks name and text values for well-formed UTF-8 and rejects control characters, but its UTF-8 loops only look at the lead-byte class and the 10xxxxxx shape of the continuation bytes. That lets through non-shortest forms (C0/C1 leads, E0 80-9F, F0 80-8F), UTF-16 surrogates written as UTF-8 (ED A0-BF) and code points above U+10FFFF (F4 90+, F5-F7 leads). Because the control-character test only sees single bytes, an overlong control character gets past it: a job-name of "a\xC0\x8Ab" (overlong LF) or "a\xE0\x80\x8Db" (overlong CR) validates, while "a\nb" is rejected. A server that vets client requests with ippValidateAttributes therefore accepts names that a lenient UTF-8 decoder further down turns back into CR/LF, and cupsUTF8ToUTF32 in transcode.c already rejects these same sequences as non-shortest, so the validator and libcups's own decoder disagree. The lead and second-byte checks in both loops now follow the RFC 3629 well-formed ranges, so valid input including U+D7FF, U+E000 and U+10FFFF is still accepted. Three testipp cases cover a valid multi-byte name, an overlong name and a surrogate in text; the last two fail without the fix.

@michaelrsweet michaelrsweet self-assigned this Oct 15, 2026
@michaelrsweet michaelrsweet added the investigating Investigating the issue label Oct 15, 2026

@michaelrsweet michaelrsweet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, if we are going down this rabbit hole we need the code to be a bit more readable... I suggest simply calculating the Unicode character and doing a range comparison. Aside from using long UTF-8 sequences and surrogates, we should also report an error if we see control characters from 0x80 to 0x9f, CR without LF (for text values), or a leading BOM - see RFC 5198 which is the governing standard for IPP name and text values.

Also, any validation error needs to reflect the issue and not just fall back on the generic "bad UTF-8 sequence" error message that we have now.

@michaelrsweet michaelrsweet added enhancement New feature or request and removed investigating Investigating the issue labels Oct 15, 2026
@michaelrsweet michaelrsweet added this to the Stable milestone Oct 15, 2026
…ibute

Move the name and text checks into ipp_validate_string, which decodes each
UTF-8 sequence to a Unicode character and range-checks it. Reject overlong
sequences, surrogates, characters above U+10FFFF, C1 controls, CR without
LF in text values and a leading BOM, each with its own error message.
@tanjiroK-coder

Copy link
Copy Markdown
Contributor Author

Reworked it. The name and text checks now go through a single ipp_validate_string helper that decodes each sequence to a code point and range-checks it, so overlong forms, surrogates and anything above U+10FFFF fall out of plain comparisons. Per RFC 5198 it also rejects C1 controls (U+0080-U+009F), CR not followed by LF in text values, and a leading BOM. Each case has its own error message (overlong, surrogate U+xxxx, out of range, C1 control U+xxxx, CR without LF, leading BOM), and the generic "bad UTF-8 sequence" is only left for truncated or malformed byte sequences. HT and bare LF in text are still accepted as before.

testipp has new cases for the C1 control, CRLF (valid), CR without LF and leading BOM, and the overlong/surrogate tests now check the error text too. All pass locally.

@michaelrsweet michaelrsweet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a little cleanup but LGTM!

@michaelrsweet
michaelrsweet merged commit b8ddfac into OpenPrinting:master Oct 16, 2026
5 of 6 checks passed
@tanjiroK-coder

Copy link
Copy Markdown
Contributor Author

Thanks for merging, and for pointing me at RFC 5198.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants