InsightSoftwareConsortium / InsightSoftwareConsortium/ITK
MetaIO: ten memory-safety defects reachable from a malformed header (Kitware/MetaIO#150)
- Dominant language
- C++
- Stars
- 1.7k
- Forks
- 748
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 64
Description
Fuzzing the vendored MetaIO found nine memory-safety defects and one unbounded allocation, all reachable from a malformed `.mha`/`.mhd` header. Full analysis, per-finding line numbers, reproducers, and the harness are in **Kitware/MetaIO#150**; this issue tracks the ITK side.
**Parked pending funding.** Filed so the work is recoverable rather than lost.
## Why this matters for ITK
`ITKIOMeta` is a default-enabled module and `MetaImageIO` is a first-class reader, so every one of these is reachable through `itk::ImageFileReader` on a file a user obtained from a scanner, a collaborator, or a public archive. The smallest reproducer is a **93-byte text header**; none require a crafted binary payload.
Two of the ten are heap-buffer-overflow **writes** with attacker-controlled length and content.
## Status
Eight of the ten have fixes written and verified locally, on an unpushed branch. Two need a decision before they can be fixed:
- **Finding 5** — `ElementType` sizes a buffer that the read loops consume as `float`, in five point-list readers. Fixing it either changes which existing files remain readable, or leaves `ElementType` decorative for those types. Compatibility call.
- **Finding 10** — a ~100-byte header requests 17 GB, and the `if (m_ElementData == nullptr)` guard cannot fire because `new` throws. There is no defensible constant cap, since large images are legitimate.
## Relationship to other issues
- **#6818** — this is the same class as the four hand-audited defects that motivated it, and a direct test of its thesis. Hand-auditing MetaIO for hours found nothing; a fuzzer found the first heap overflow in **90 seconds**, and eight more over five rounds. MetaIO is a reasonable first target for the CI leg proposed there: 28k lines, self-contained, no ITK dependency, builds standalone in under a minute.
- **#6694** — re-vendoring is already tracked. Any MetaIO fixes land upstream first and reach ITK through `Modules/ThirdParty/MetaIO/UpdateFromUpstream.sh`, so that path is the delivery mechanism here too.
- **#5084** — OpenSSF Scorecard *Fuzzing: 0*. The harness in MetaIO#150 is a working starting point.
What the existing safety nets did and did not catch
| Check | Result |
|---|---|
| MetaIO's 15 tests under ASan + UBSan | **clean** — they only feed well-formed files |
| `-Wall -Wextra` | 3 warnings, none related |
| CodeQL `cpp-security-and-quality` (159 results) | **missed findings 1-9** |
| libFuzzer + ASan/UBSan, 5 rounds | **found all 10** |
CodeQL missing them is worth understanding rather than treating as a tooling failure. The unbounded copy loop has no constant to compare a length against, and the `strncat` misuse is syntactically well-formed — the wrong argument is a plausible-looking named constant in the wrong position. CodeQL did independently flag finding 10, plus four genuine `new[]`/scalar-`delete` mismatches in `metaTransform.cxx`. The tools are complementary; neither substitutes for the other.
Re-fuzzing after each fix was necessary, not thorough
Findings arrived over five rounds because fixes changed what was reachable:
- Finding 6 exists *because* finding 3's first fix bounded `length` where the matrix path consumes `length²` — a correct-looking bound that was dimensionally wrong, which the fuzzer walked into within 35,000 executions.
- Finding 9 only surfaced once earlier crashes stopped terminating inputs first.
- Coverage rose 1713 → 2084 blocks across the rounds as fixes unblocked deeper paths.
Round 5 found no new memory corruption in ~115,000 executions per worker. That is the yield beginning to flatten, not evidence the code is clean.
Picking this back up
Local state is a git worktree on branch `harden` off the MetaIO PR #149 branch: eight fixes, a `testMeta16MalformedHeader` regression test covering seven reduced cases (verified to fail without the fixes), libFuzzer harnesses for `MetaImage` and `MetaScene`, an evolved corpus, and a CodeQL database. Unpushed.
Everything needed to rebuild from scratch is in MetaIO#150 — harness source, build flags, and the reproducers as `printf` commands — so none of this depends on that local tree surviving.
Order of work when resumed: settle findings 5 and 10, land one upstream MetaIO PR stacked on Kitware/MetaIO#149, then re-run `UpdateFromUpstream.sh` here. The standing CI leg from #6818 is the thing that keeps it from regressing.
Contributor guide
Research direction
Start with Kitware/MetaIO#150, including its reproducers and libFuzzer harnesses, then review the MetaIO changes and the open decisions for findings 5 and 10. The delivery path is Modules/ThirdParty/MetaIO/UpdateFromUpstream.sh after an upstream fix lands. Done means resolving both decisions, landing the remaining MetaIO fixes upstream, and re-vendoring them into ITK.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- security, testing
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100