Protocol parser vulnerability fixes

[neilalexander]

There are a number of severe problems in the protocol parser that can result in either out-of-bounds reads, out-of-bounds writes, stale/uninitialised memory disclosure or memory corruption bugs that can lead to repeater crashes. Some of these conditions can be tripped easily by broadcasting truncated or malicious frames within reception range of a repeater. Others require a bit more work, e.g. requiring valid MACs.

Some of these faulty paths may also affect companion devices, but I have focused specifically on repeaters here given that they are frequently installed in hard-to-reach places and can be difficult to recover.

Issues include:

• Out-of-bounds reads from packets of any type with truncated headers, transport codes or outer paths, or malicious packets declaring more path bytes than they contain.

• Stale memory reads and potentially out-of-bounds reads and writes from REQ, RESPONSE, TXT_MSG, GRP_TXT, GRP_DATA and PATH packets with truncated ciphertext or ciphertext lengths that aren’t multiples of 16.

• Stale memory reads from ANON_REQ packets with truncated or non-block-aligned ciphertext.

• Stale memory disclosure from malicious, validly encrypted ANON_REQ REGIONS, OWNER or BASIC/clock requests declaring more reply-path bytes than they contain.

• Stale memory disclosure and potentially out-of-bounds reads from malicious, validly encrypted PATH packets declaring more path bytes than their plaintext contains, or omitting the following extra-type byte.

• Stale memory reads from truncated ACK packets containing fewer than four payload bytes.

• An out-of-bounds write from malicious oversized plain ACK packets when a forwarding node expands a maximum-sized ACK into a MULTIPART ACK.

• Stale memory reads and potentially out-of-bounds reads or crashes from truncated TRACE packets whose missing nine-byte prefix causes a length underflow, or malicious TRACE packets containing incomplete hashes.

• Stale memory reads from ADVERT packets with truncated public-key or timestamp fields, and out-of-bounds reads relative to application-data length when optional coordinates or feature fields are truncated.

• Out-of-bounds writes and possible crashes from malicious oversized advertisement names passed directly to AdvertDataParser.

• Stale memory reads from empty or truncated CONTROL packets.

Many of these boundary violations could have been caught with sufficient testing combined with AddressSanitizer and UndefinedBehaviorSanitizer. I didn’t look at MemorySanitizer but that would probably also be useful here too.

I have added fixes for all of the above cases, including some new unit tests and a new ASAN test target that runs with both AddressSanitizer and UndefinedBehaviorSanitizer.

The Packet::readFrom() function now centralises common frame and header length validation for all packet types. Type-specific field length validation has been factored out into a new isValidPayload() function, and some unsafe logic has been deduplicated from Dispatcher::tryParsePacket(). There are also a couple new shared helpers for decrypted PATH data and more.

This is public disclosure of GHSA-2fvm-7f8c-957x three months after it was originally filed. At the time of writing, these changes merge cleanly onto main as well as dev.