Skip to content

fix(#2538): refuse a JSON parametric curve without exactly its parameters - #2543

Merged
xsscx merged 1 commit into
masterfrom
fix/2538-parametric-curve-params
Sep 13, 2026
Merged

fix(#2538): refuse a JSON parametric curve without exactly its parameters#2543
xsscx merged 1 commit into
masterfrom
fix/2538-parametric-curve-params

Conversation

@colourbill-ctrl

Copy link
Copy Markdown
Contributor

Closes #2538.

The problem

CIccTagJsonParametricCurve::ParseJson() accepted a parametric curve that supplied fewer parameters than its function type requires, and returned true with the rest of m_dParam uninitialised. SetFunctionType() allocates m_dParam with a bare new[] and fills nothing; the reader wrote only min(supplied, required) entries, and none at all when params was missing or not an array. Write() then encoded whatever the heap held.

There were two ways in:

entry example before this PR
short, missing or non-array params functionType 4, params [2.4] accepted; 6 of 7 parameters uninitialised
functionType outside 16 bits 65540 or -65532, seven params accepted as type 4 with all 7 uninitialised: the count came from a switch on the int (no case, count 0) while SetFunctionType() got the value narrowed to 4

It also silently truncated a longer array.

The XML twin, CIccTagXmlParametricCurve::ParseXml(), has required the exact count since the 2015 import and has bounded the function type since #1851. On the same build, iccFromXml writes no profile for the short, empty, long and 65540 variants. The binary Read() fills every parameter.

The change

  • functionType is read as icUInt16Number. jsonToValue range-checks the conversion, so a value that does not fit the field is refused.
  • The count comes from GetNumParam(), so SetFunctionType()'s table is the only one. The count enforced is, by construction, the size of the allocation.
  • params must be an array of exactly that count, matching XML. A longer array is now refused. iccToJson always emits exactly GetNumParam() values, and no tracked JSON carries a parametric curve.
  • Unknown function types (5 and up) are still accepted with zero parameters, as the binary reader keeps them.

Tests

New: .github/ci/regression/parametric-curve-params-json.cppiccdev.parametric-curve-params-json. It drives the reader directly and asserts both the return value and every parameter of each accepted curve. Neither depends on heap contents, so the red shows on a plain build.

  • Red: against master fb5f65d0, exactly the 7 refusal cases fail, each observing true.
  • Mutation: !=< on the count fails only the eight-parameter case. Restoring the int read fails only the 65540 and -65532 cases.

The maintainer's Valgrind path, with the short-params document from the issue, valgrind --track-origins=yes --error-exitcode=86:

build result
master fb5f65d0, Release 27 uninitialised-value errors, exit 86, CIccTagParametricCurve::ApplyValidate
this PR 5c48c565, Release 0 errors; refused with "params count does not match functionType"

Pre-flight

All measured on the commit this PR carries.

lane result
clang ASan/UBSan Debug, full suite (274) 272 pass, 1 skip, 1 failure: iccdev.spectral-tiff-preview, which needs a local imagecodecs Python module
clang ASan/UBSan Debug, -L json 27/27
strict clang 21 Release, -Werror 0 warnings, targeted build (IccJSON, iccFromJson, iccToJson, the new test)
GCC 15.2 container, strict enabled + LTO 0 warnings, targeted build, same targets

The full strict builds were killed by the local harness's memory guard, so the strict evidence covers the changed library and its tools, not every target. Dispatched ci-pr-action with ci_scope=source before opening: run 34732433437 concluded success, 14 jobs green and 2 skipped by design. iccdev.parametric-curve-params-json is logged Passed in GCC Strict ASAN+UBSAN Debug (job 103657605039), Windows MSVC (103657605001) and Windows ClangCL (103657604974). The GCC 15.2 LTO and macOS jobs are green but log no CTest line for it.

Not in this diff, noted only: a float functionType such as 4.7 is read as 4, and parameters pass through a double to float cast.

@github-actions github-actions Bot added Testing CTest, regression, or test coverage Source C or C++ source code changes Configuration Repository, CMake, YAML, JSON, or tool configuration Build Build system, CMake, compiler, or packaging JSON ICC JSON parser, config, or round-trip behavior pending CI checks still running passed All CI checks passed and removed pending CI checks still running labels Sep 13, 2026

@xsscx xsscx 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.

2026-09-13 03:04:55 UTC

@xsscx xsscx self-assigned this Sep 13, 2026
@xsscx xsscx added this to the v2.3.2.4 milestone Sep 13, 2026
@xsscx
xsscx merged commit 67e90cf into master Sep 13, 2026
37 checks passed
@xsscx
xsscx deleted the fix/2538-parametric-curve-params branch September 13, 2026 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build Build system, CMake, compiler, or packaging Configuration Repository, CMake, YAML, JSON, or tool configuration JSON ICC JSON parser, config, or round-trip behavior passed All CI checks passed Source C or C++ source code changes Testing CTest, regression, or test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bisect: 60bbb8c CIccTagJsonParametricCurve::ToJson() | Asan poison encoded to 0xffffa0a1, returns as 0xbebebe00

2 participants