Fix: signed->unsigned SeamlessIndicator conversion in image ParseXml (#1342, #1343)#1344
Merged
xsscx merged 1 commit intoJun 14, 2026
Conversation
…ml (InternationalColorConsortium#1342, InternationalColorConsortium#1343) CIccTagXmlEmbeddedHeightImage::ParseXml() and CIccTagXmlEmbeddedNormalImage::ParseXml() read the "SeamlessIndicator" attribute with atoi() (a signed int) and assigned it directly to the unsigned 32-bit member m_nSeamlesIndicator. A negative attribute value wrapped to a huge unsigned value through the implicit int -> icUInt32Number conversion, which UBSan's implicit-integer-sign-change check flags as a runtime error (IccTagXml.cpp:5548 InternationalColorConsortium#1342 / :5651 InternationalColorConsortium#1343). Both sites now parse into a signed temporary and floor any negative (invalid) value to 0 before the unsigned assignment, so the stored indicator is always a well-defined non-negative number. Positive values pass through unchanged (atoi already capped at int range, so the XML path could never represent a value above INT_MAX); only the invalid negative case -- which previously triggered the UB -- now resolves to 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
colourbill-ctrl
requested review from
ChrisCoxArt,
maxderhak and
xsscx
as code owners
This was referenced Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1342 and #1343 (same root cause, same fix pattern), both UBSan
implicit-integer-sign-changefindings from the xsscx fuzz/QA wave (bisected tobab6678).CIccTagXmlEmbeddedHeightImage::ParseXml()(IccTagXml.cpp:5548, #1342) andCIccTagXmlEmbeddedNormalImage::ParseXml()(IccTagXml.cpp:5651, #1343) read theSeamlessIndicatorXML attribute withatoi()(signedint) and assigned it directly to the unsigned 32-bit memberm_nSeamlesIndicator. A negative attribute value wrapped to a huge unsigned value via the implicitint -> icUInt32Numberconversion:Fix
Both sites parse into a signed temporary and floor any negative (invalid) value to 0 before the unsigned assignment, so the stored indicator is always a well-defined non-negative number. Positive values pass through unchanged —
atoialready capped atintrange, so the XML path could never represent a value aboveINT_MAX; only the previously-UB negative case now resolves to 0. The binaryRead()path (Read32) is unaffected.Verification
Built
iccFromXmlwith-fsanitize=implicit-conversion -fno-sanitize-recover=all(clang-18):...EmbeddedHeightImage-...Line5548.xml(SeamlessIndicator="-98")...EmbeddedNormalImage-...Line5651.xml(SeamlessIndicator="-5655")A valid positive
SeamlessIndicator(e.g.7) still parses and stores unchanged.Notes
IccXML/), no fork-gate split needed..github/+Build/Cmake, which the fork-PR automation gate blocks). Merge this fix first; the test PR goes green once this lands.🤖 Generated with Claude Code