-
Notifications
You must be signed in to change notification settings - Fork 43
Fix: CIccTagLut16::Read() UB #223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
xsscx
merged 26 commits into
InternationalColorConsortium:master
from
ChrisCoxArt:CIccTagLut16_Read_Fix
Nov 24, 2025
Merged
Fix: CIccTagLut16::Read() UB #223
xsscx
merged 26 commits into
InternationalColorConsortium:master
from
ChrisCoxArt:CIccTagLut16_Read_Fix
Nov 24, 2025
+3
−3
Conversation
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
Lots of unused parameters and variables More than a few values set but never used Lots of bad switch() statements that don't have a default or use all enum values Lots of bad cases using values that are not part of the enum Lots of sprintf without length check! The actual errors are difficult to see among all the warning noise.
spectral m_Range was never copied correctly, code probably needs more test cases.
sigh, someone played fast and loose with buffer management. Need to make sure sizes are passed in, or std::string is used (slow, but less error prone)
and I need to check in these changes before I grab dinner
more of the same errors and deprecated functions, unused parameters and variables, etc.
Also update several utility functions, for safety. It still isn't 64 bit file offset ready for ICC profiles, but at least we get rid of a ton of casting errors and warnings, and are more likely to catch overflows.
Down to 87 warnings (after fileIO refactor also applied)
fix a bunch of variables shadowing local variables That sort of code always get confused. Try to fix some macros that caused shadow variables, add notes to eventually remove those macros.
refactor file IO to use size_t
fixing up enum abuse
Not yet tested properly, because I can't find a profile that hits that code path
It was previously only noted in the CMake files.
xsscx
approved these changes
Nov 24, 2025
Member
xsscx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR223 Repro
Reported in #213
Host
Last Updated: Sun Nov 23 08:17:42 PM EST 2025
Linux 6.6.87.2-microsoft-standard-WSL2 #1 SMP PREEMPT_DYNAMIC Thu Jun 5 18:30:46 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Verify PR223 Source
git status
On branch pr-223
git log
commit 4b2cbf6e1ae94b30d4cb4cfe23e91780b3ec2f21 (HEAD -> pr-223)
Merge: ccd0526 b34b7b2
Author: Chris Cox <ccox@comcast.net>
Date: Sun Nov 23 17:08:27 2025 -0800
Merge branch 'InternationalColorConsortium:master' into CIccTagLut16_Read_Fix
PR Configure, Build & Test
git clone https://github.com/InternationalColorConsortium/iccDEV.git
cd iccDEV
git fetch origin pull/223/head:pr-223
git checkout pr-223
cd Build
cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_CXX_FLAGS="-g -fsanitize=address,undefined -fno-omit-frame-pointer" -Wno-dev Cmake/
make -j$(nproc)
cd ../Testing/
echo "=== Updating PATH ==="
for d in ../Build/Tools/*; do
[ -d "$d" ] && export PATH="$(realpath "$d"):$PATH"
done
wget https://github.com/xsscx/Commodity-Injection-Signatures/raw/refs/heads/master/graphics/icc/Argyll_V302_null_byte_read-icmTable_setup_bwd-cve-2023-46867-variant-argyle-001.icc
iccDumpProfile -v Argyll_V302_null_byte_read-icmTable_setup_bwd-cve-2023-46867-variant-argyle-001.icc
Expected Output
Built with IccProfLib version 2.3.1
Unable to parse 'Argyll_V302_null_byte_read-icmTable_setup_bwd-cve-2023-46867-variant-argyle-001.icc' as ICC profile!
Validation Report
-----------------
Profile has Critical Error(s) that violate ICC specification
NonCompliant! - Bad Profile ID
Error! - - AToB0Tag - Tag has invalid structure!
Error! - - BToA0Tag - Tag has invalid structure!
Error! - - AToB0Tag - Tag has invalid structure!
Error! - - AToB2Tag - Tag has invalid structure!
EXIT -1
Member
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
CVE Requested
Maintainer indicates a CVE has been Requested
Merged
Merged
PR
Pull Request
Security
Security Related
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.
Fixes #213