The guard that was there eight times and missing once

TL;DR: Fuzzing libE57Format — the E57 3D point-cloud reader used by CloudCompare, AliceVision, FreeCAD and PDAL — turned up a null-pointer dereference while parsing the XML section of a crafted file. Reported 25 August, fixed and merged upstream by the 28th as 93ddd12f. Three things about getting there were more interesting than the bug itself.

The bug

CompressedVectorNodeImpl holds a prototype_ (the record layout) and a codecs_ vector. Both are attached after construction, via setPrototype() and setCodecs(), so both are legitimately null for part of the object’s life.

The file knows this. Every use of those members is guarded:

if ( prototype_ ) { ... }      // :52
if ( codecs_ )    { ... }      // :97
if ( prototype_ ) { ... }      // :178
if ( codecs_ )    { ... }      // :184
                               // ...and at 227, 231, 244, 253

Eight guards. And then, in isTypeEquivalent():

// Prototypes and codecs must match ???
if ( !prototype_->isTypeEquivalent( cvi->prototype_ ) )

No check — and note it isn’t the receiver that bites, it’s the argument. cvi->prototype_ is a null shared_ptr from a CompressedVector element that a crafted XML section left without a prototype, and StructureNodeImpl::isTypeEquivalent promptly does ni->type() on it.

The author’s own //??? comments sit two functions away, on getPrototype() and getCodecs(): “check defined”. The concern was noted and never followed through.

The fix extends the existing idiom rather than inventing a new one — a missing prototype means “not equivalent”, the same answer a mismatch already gives:

if ( !prototype_ || !cvi->prototype_ ) { return ( false ); }

The corpus had to build itself

The upstream test-data repository ships fifteen .e57 files. They exercise a narrow slice of the format: mostly float and double cartesian points. A campaign seeded with just those ran for an hour and forty minutes and found nothing.

E57’s interesting property is that an XML section describes the layout of the binary section. Every binary field has an attacker-controlled XML twin — the decoder derives its bit widths from XML-declared minima and maxima via bitsNeeded(min, max). That’s the surface, and the shipped files barely touch it.

So the corpus was generated by driving libE57Format’s own Writer API across the axes the samples miss: every node type, bit widths chosen to land on 1, 8, 15, 17, 31 and 33 bits, cartesian versus spherical geometry, invalid-state fields, intensity/colour/row-column/return/timestamp combinations, zero-point and four-thousand-point scans, and Image2D sections.

Two constraints shaped that, and both were discovered by reading the library’s validation rather than by guessing after the errors appeared. pointRangeNodeType cannot be Integer — the API forbids it outright — so bit width has to be driven through ScaledInteger’s scale, since the underlying integer range is (max - min) / scale. And optional-field bounds must be declared or the writer rejects the values; those declared ranges are themselves part of what the decoder reads back.

Union coverage went from 17,856 tuples to 24,039. The crash arrived about nine hours in.

The reproducer wasn’t real yet

This is the part worth carrying to other targets.

E57 files are written in 1024-byte pages: 1020 bytes of data plus a four-byte CRC-32C. The harness ran with ChecksumNone, because with verification on, every mutation dies at the page check before reaching a decoder.

That’s the right call for throughput — and it means the crash the fuzzer produced was not a real-world bug. Under ChecksumAll, which is what every actual caller uses by default, the file is cleanly rejected as corrupt. Reported as-is, the maintainer could not have reproduced it.

Repairing the page checksums fixes that, and the repaired file still crashes with verification enabled. One wrinkle on the way: the first repair attempt wrote the CRC little-endian and the file stayed rejected. Rather than guess, I validated the routine against a known-good file — 366 of 366 pages matched once it was stored big-endian.

The general lesson: if your harness relaxes an integrity check to make fuzzing possible, every crash it finds is a candidate, not a finding, until you restore the check. Two lines in a triage checklist, and the difference between a credible report and a wasted maintainer’s afternoon.

One guard with no reproducer

The patch also guards codecs_, one line below the crashing site, which had the identical gap. There was no reproducer for it at the time — it went in purely because the sibling had the same shape.

Over the following days the campaign produced fourteen more crashes. All the same root cause, and one of them reached codecs_, arriving via VectorNodeImpl::isTypeEquivalent rather than StructureNodeImpl because codecs_ is a vector node. A fix targeting only the crash that had been observed would have left it open.

Not a vulnerability

Andy Maloney’s response to the report:

I consider these kinds of issues (malformed file causing a crash) bugs, not security issues.

That’s his call, and a defensible one for a file-format library. No advisory, no CVE, no embargo — so this is published as an ordinary bug-fix note rather than a security advisory. He wrote a test case, attached it to the pull request, and merged the fix the same day.

Reported to merged in three days.