BUG: Backport GDCM CVE-2026-3650 fix

[thewtex]

Backport of GDCM PR malaterre/GDCM#214 (commit 23bca9286a7efe8be97d67015aa280138fa8d4b1).

A crafted DICOM file could specify an arbitrarily large Value Length field (up to ~4 GB), causing ByteValue::SetLength() to attempt a massive memory allocation before any stream data is read. This enables denial-of-service via memory exhaustion.

Add stream-size validation in ExplicitDataElement::ReadValue(), ImplicitDataElement::ReadValue(), Fragment::ReadValue(), and Fragment::ReadBacktrack(). Before allocating a ByteValue, the code now compares the declared VL against the remaining bytes in the stream via tellg()/seekg(). Non-seekable streams skip the check gracefully.

Also fix out-of-bounds array accesses in SequenceOfFragments where bv->GetLength() - N was used without verifying minimum length, affecting lines that use gdcmAssertAlwaysMacro (active in release).

The TestCVE20263650 test added upstream is not included, as ITK's GDCM subtree does not vendor the Testing/ directory.

[greptile-apps]

Greptile Summary

This PR correctly backports GDCM's upstream CVE-2026-3650 fix (PR #214, commit 23bca92). It adds stream-size validation via tellg()/seekg() before any large ByteValue allocation in four locations (ExplicitDataElement::ReadValue, ImplicitDataElement::ReadValue, Fragment::ReadValue, Fragment::ReadBacktrack), and adds short-circuit length guards before out-of-bounds array accesses in SequenceOfFragments. The implementation is consistent with the upstream fix and the confirmed behavior of gdcm_assert (which always evaluates and throws, not a no-op in release) and gdcmAssertAlwaysMacro (throws in NDEBUG mode).

Confidence Score: 5/5

Safe to merge — correctly backports upstream CVE-2026-3650 fix with no regressions identified.

All four changed files apply the same well-structured upstream fix pattern. The readvalues guard in ExplicitDataElement is confirmed correct (SetValueFieldLength with readvalues=false calls SetLengthOnly, skipping allocation). The gdcm_assert bounds fixes in SequenceOfFragments are effective in both debug and release because gdcm_assert always evaluates and throws. No P0/P1 findings remain.

No files require special attention.

Important Files Changed

Filename	Overview
Modules/ThirdParty/GDCM/src/gdcm/Source/DataStructureAndEncodingDefinition/gdcmExplicitDataElement.txx	Adds seekable-stream size validation (tellg/seekg) before ByteValue allocation in ReadValue(); guard correctly scoped to readvalues=true since SetValueFieldLength skips the allocation when readvalues=false (calls SetLengthOnly).
Modules/ThirdParty/GDCM/src/gdcm/Source/DataStructureAndEncodingDefinition/gdcmImplicitDataElement.txx	Adds identical stream-size validation, correctly gated on both !ValueLengthField.IsUndefined() and readvalues to avoid false positives for undefined-length SQ elements in implicit encoding.
Modules/ThirdParty/GDCM/src/gdcm/Source/DataStructureAndEncodingDefinition/gdcmFragment.h	Adds stream-size checks in ReadValue() and ReadBacktrack() before ByteValue::SetLength(); no readvalues parameter on Fragment::ReadValue so the check is unconditional and appropriate.
Modules/ThirdParty/GDCM/src/gdcm/Source/DataStructureAndEncodingDefinition/gdcmSequenceOfFragments.h	Adds length pre-conditions to three assert macros; all three are safe because gdcm_assert always evaluates (throws on failure in both debug/release) and gdcmAssertAlwaysMacro also throws/aborts in release, so the subsequent SetByteValue underflow is unreachable when the guard fires.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Read DICOM Element Header\nTag + VL] --> B{VL undefined\nor readvalues=false?}
    B -- Yes --> F[SetValueFieldLength\nSetLengthOnly — no alloc]
    B -- No --> C[tellg → cur]
    C --> D{cur == -1?\nStream not seekable}
    D -- Yes --> F
    D -- No --> E[seekg end → tellg end\nseekg cur — restore]
    E --> G{is.good &\nend - cur < VL?}
    G -- No / seek failed --> F
    G -- Yes: VL exceeds stream --> H[throw Exception\nDoS prevented]
    F --> I[ByteValue::SetLength\nAllocate buffer]
    I --> J[Read data from stream]

    style H fill:#f66,color:#fff
    style I fill:#6a6,color:#fff

Loading

_{Reviews (1): Last reviewed commit: "BUG: Backport GDCM CVE-2026-3650 fix" | Re-trigger Greptile}

[blowekamp]

Has the fix been applied to the main branch yet? I have concerns with release-5.4 not being merged into release and main after updates and accumulating more conflicts or other issues.

This changes look fine.

[hjmjohnson]

I was concerned about the lack of testing, but the test exists upstream where it is really needed.

[hjmjohnson]

@thewtex This is a CDash hiccup holding this up.

[thewtex]

Has the fix been applied to the main branch yet?

I'll create another PR for the main branch version.