Skip to content

Fix null dereference comparing CompressedVector prototypes - #356

Merged
asmaloney merged 2 commits into
asmaloney:masterfrom
glitchfox:fix-null-prototype-comparison
Aug 28, 2026
Merged

Fix null dereference comparing CompressedVector prototypes#356
asmaloney merged 2 commits into
asmaloney:masterfrom
glitchfox:fix-null-prototype-comparison

Conversation

@glitchfox

Copy link
Copy Markdown
Contributor

Fixes the null dereference from GHSA-f7fw-gw7v-2h7g.

prototype_ and codecs_ are set after construction via setPrototype() /
setCodecs(), so both can legitimately be null. Every other use in
CompressedVectorNodeImpl.cpp checks for that (lines 52, 97, 178, 184, 227, 231,
244, 253) — isTypeEquivalent() was the one place that didn't, so passing a null
prototype into StructureNodeImpl::isTypeEquivalent() dereferenced it at
ni->type().

A missing prototype or codecs is now treated as "not equivalent", which is the
same answer a mismatch already gives.

codecs_ is guarded as well: it has the identical gap one line later. Our
reproducer happens to trip the prototype path first, but a later fuzzing run did
produce an input that reaches the codecs_ line, so both are needed.

Verified locally against 21c2943:

  • 6 crashing inputs from the fuzzing run are all cleanly rejected after the patch
    (with E57_ERROR_HOMOGENEOUS_VIOLATION, i.e. the real structural problem is now
    reported), and all 6 still crash without it.
  • No behaviour change on valid files: 46 files — the 15 in libE57Format-test-data
    plus 31 generated through the Writer API across node types, bit widths,
    cartesian/spherical geometry, optional-field combinations, scan counts and
    Image2D sections — give identical open/reject results before and after.

Happy to adjust anything. Let me know when you've added your test case and I'll
rebase if needed.

glitchfox and others added 2 commits August 28, 2026 09:34
CompressedVectorNodeImpl's prototype_ and codecs_ are set after construction
via setPrototype()/setCodecs(), so both can be null. Every other use in this
file checks for that, but isTypeEquivalent() did not -- passing a null
prototype into StructureNodeImpl::isTypeEquivalent() dereferences it via
ni->type().

A malformed XML section can produce such a node, which crashes while parsing.

Treat a missing prototype or codecs as not equivalent, which is the same
result a mismatch already gives. codecs_ is guarded too since it has the same
issue one line later.
@asmaloney
asmaloney merged commit 93ddd12 into asmaloney:master Aug 28, 2026
12 checks passed
@asmaloney

Copy link
Copy Markdown
Owner

Thanks Benjamin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants