Skip to content

reject invalid UTF-8 sequences in _zip_guess_encoding - #547

Merged
0-wiz-0 merged 2 commits into
nih-at:mainfrom
arshsmith1:utf8-strict-validation
Aug 12, 2026
Merged

reject invalid UTF-8 sequences in _zip_guess_encoding#547
0-wiz-0 merged 2 commits into
nih-at:mainfrom
arshsmith1:utf8-strict-validation

Conversation

@arshsmith1

Copy link
Copy Markdown
Contributor

The UTF-8 detection in _zip_guess_encoding only checks the length pattern of the lead byte and the high bits of the continuation bytes, so it treats overlong encodings, surrogate halves (U+D800 to U+DFFF), and code points above U+10FFFF as valid UTF-8. That lets a filename or comment marked UTF-8 (general purpose bit 11) slip past the validation zip_open performs, even though libzip means to reject invalid UTF-8 there with ZIP_ER_INCONS. This decodes the code point while scanning the continuation bytes and rejects those three cases so the existing check fires; valid multibyte names are unaffected. Added a regress case that opens a crafted archive whose name is the encoded surrogate half U+D800.

@0-wiz-0

0-wiz-0 commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thank you, good points.
I think the codepoint is incorrect for 1-byte characters, and it would be nicer to use one (or two) named constants instead of 0x3f. Would you be willing to adapt the code?

@arshsmith1

Copy link
Copy Markdown
Contributor Author

Happy to. I pulled the lead-byte extraction into each length branch so the low bits are masked off with the matching UTF_8_LEN_*_MASK, which drops the shifted-0x3f trick that was misbehaving in the 1-byte case. The continuation payload now uses a named UTF_8_CONTINUE_VALUE_MASK instead of the bare 0x3f. Full regress suite still passes, and the surrogate test still rejects as before.

@0-wiz-0
0-wiz-0 merged commit 009b63d into nih-at:main Aug 12, 2026
1 check was pending
@0-wiz-0

0-wiz-0 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Thank you, merged!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants