packeteer reads and writes pcapng, so it can open a capture somebody else made. That sentence is the whole threat model: a capture file is untrusted input, and the reader is the code that touches it first.

I had written the reader, tested it, and used it. Then I wired up libFuzzer harnesses for every hand-rolled decoder in the project, plus the pcapng reader and the full summarize_packet() pipeline. The reader harness found a bug on its first run.

The bug

A pcapng file is a sequence of blocks. Each block header carries a 32-bit total length, and Reader::next_packet() used it the obvious way:

read the block header
allocate the block's claimed body size
read the body into it

The claimed size is a 32-bit field read straight out of the file. Nothing had checked it against how many bytes were actually left. A 40-byte file could declare a 4 GB block, and the reader would try to allocate 4 GB before discovering there was nothing to put in it.

That is not a crash in the interesting sense - nothing is corrupted, no attacker-controlled pointer is dereferenced. It is a denial of service, and for a tool whose job is opening files from other people it is squarely in scope. Hand somebody a capture, they open it in your analyzer, the analyzer dies.

Why review had not caught it

Because the code is correct-looking. Reading a length and allocating it is what every reader does. The missing step is not a wrong line, it is an absent one, and absent lines do not draw the eye - especially in code you wrote yourself and have read past a dozen times.

The fuzzer does not read. It generates a header with a large length field and a file with nothing after it, which is a case a person writing test fixtures does not think to write, because the person is thinking about valid pcapng files with slight defects rather than about arbitrary bytes.

The fix

A 1 MiB cap on the block body size, checked before the allocation.

The number is deliberately generous rather than tight. reader.hpp is scoped to pair with packeteer’s own writer, whose packets are capped at a 65535 snaplen, so 1 MiB is roughly sixteen times more headroom than any block this project produces needs. A tighter bound would be defensible for a self-contained tool and wrong for one that might read a legitimately unusual capture from elsewhere.

Then two things, both of which matter:

  • A unit test, so the case is described in the test suite in a form a human reads.
  • A re-fuzz of the exact crashing input, so the thing that found it agrees it is gone. A unit test asserts you fixed what you understood. Re-running the fuzzer on the original reproducer asserts you fixed what actually happened.

The session in numbers

Eight harnesses, roughly 23 million total executions, one bug, zero remaining crashes. Every hand-rolled decoder is covered: Ethernet, IPv4, IPv6, TCP with reassembly, UDP, DNS, HTTP, TLS, the checksum code, the pcapng reader, and the whole summarize pipeline end to end.

One bug in 23 million executions sounds like a poor yield until you notice where it was: not in a dissector, where I had been careful because I knew the input was hostile, but in the reader, where the input is a file and files feel like something you own. The bytes do not care how they arrived.

What I took from it

The mental model that let this bug exist was that a capture file is data and a packet is input. They are both input. The file has a header format with length fields authored by whoever handed it to you, exactly like the packets inside it, and it deserves the same suspicion.

Second: fuzz the boundary code first, not last. I built the harnesses after the dissectors were done, on the assumption that the parsers were the risky part. The parsers were fine. The thing wrapping them was not.