net/packet: don't panic in Payload when dataofs is past length - #21232

Fixes #21231

(*Parsed).Payload says it returns nothing rather than crashing on a truncated packet, but its guard compares both length and dataofs against len(b) while the slice it returns is b[dataofs:length]. What actually has to hold is dataofs <= length, and those two are independent: length is the IPv4 total length header field, dataofs is derived from the sub-protocol header, and decode4 never checks that the declared total length covers the transport header.

A 28-byte IPv4/UDP packet declaring a total length of 20 decodes to length=20 dataofs=28 len(b)=28, and Payload then evaluates b[28:20]:

panic: runtime error: slice bounds out of range [28:20]

tailscale.com/net/packet.(*Parsed).Payload(...)
	net/packet/packet.go:472

ICMPv4 and TCP get there the same way, with 24- and 40-byte packets. The test I added covers all three; it panics on main and passes with the extra condition.

I found this by fuzzing Decode and then calling the accessors on the parsed result. The net/packet fuzz target in #21123 only calls Decode, which is why it did not surface.

I went looking for a caller that can be driven into this from the network and could not prove one. filterPacketInboundFromWireGuard reaches Payload through isSelfDisco for every inbound UDP packet, but wireguard-go truncates decrypted packets to the declared IP length first, so on that path dataofs > length implies dataofs > len(b) and the existing guard already catches it. So I am sending this as a robustness fix for an exported parser whose documented contract is that it will not crash, not as a fix for a reachable crash. If you would rather have the invariant enforced in decode4 (rejecting the packet as Unknown when the declared length does not cover the sub-protocol header) than repaired at the accessor, I am happy to redo it that way.

While I was there I also noticed that (*Parsed).Transport can panic the same way, because decode4 assigns q.subofs before validating it against q.length, so the rejection path leaves subofs > len(b) with IPVersion still set. I left that out of this change to keep it to one thing; tell me if you want it here or in its own issue.

AI tools used