noi/gopacket

Fork 0

layers: 26 decoders index past their own length guard on short input — a 15-byte frame reaches most of them #11

New issue

Open

opened 2026-08-26 10:02:38 +00:00 by claude · 2 comments

claude commented

2026-08-26 10:02:38 +00:00

Collaborator

Severity: medium · layers/ — 26 distinct decoders, see table

Not a single bug: a class of bug, found by a deterministic sweep rather than by fuzzing. Every one of these decoders validates a minimum length and then indexes past it, or derives an offset with arithmetic that wraps.

Method

For every registered gopacket.LayerType, feed buffers of every length 0–80 filled with each of six byte patterns (00, ff, inc, 80, 0f, 3f), with SkipDecodeRecovery: true so panics surface, then call Layers(), LayerContents() and LayerPayload() on the result.

swept 2000 registered layer types x 81 lengths x 6 patterns = 972000 probes
layers with at least one panic: 26
distinct (layer, panic shape) pairs: 39

The whole harness is about 60 lines and runs in under a minute. It is attached below as a suggested CI test.

Results

layer	smallest trigger	panic
ARP	8 B `ff`	`slice bounds out of range [8:7]`
ARP	8 B `80`	`slice bounds out of range [:136] with capacity 8`
GRE	0 B	`index out of range [0] with length 0`
GRE	2 B `00`	`slice bounds out of range [:4] with capacity 2`
MPLS	0 B	`slice bounds out of range [:4] with capacity 0`
PPPoE	0 B / 6 B `ff`	`[:4] with capacity 0` / `[6:5]`
EtherIP	0 B / 1 B	`index out of range [0]` / `[:2] with capacity 1`
IPSecAH	12 B `00`	`slice bounds out of range [12:8]`
IPSecESP	0 B	`slice bounds out of range [:4] with capacity 0`
UDPLite	0 B	`slice bounds out of range [:2] with capacity 0`
SCTP	13 B `00`	`slice bounds out of range [:4] with capacity 1`
RUDP	0 B	`slice bounds out of range [:6] with capacity 0`
GTPv1U	9 B `00`	`index out of range [1] with length 1`
Geneve	7 B `00`	`slice bounds out of range [:8] with capacity 7` (see #8)
ERSPAN Type II	0 B / 1 B	`index out of range [0]` / `[:2] with capacity 1`
EthernetCTP	0 B	`slice bounds out of range [:2] with capacity 0`
CiscoDiscovery	0 B	`slice bounds out of range [:4] with capacity 0`
Linux SLL	16 B `ff`	`slice bounds out of range [6:5]`
Linux SLL	16 B `inc`	`slice bounds out of range [:1035] with capacity 16`
RadioTap	8 B `inc`	`index out of range [8] with length 8`
RadioTap	14 B `inc`	`slice bounds out of range [770:14]`
SFlow	0 B	`slice bounds out of range [4:0]`
SFlow	24 B `ff`	`slice bounds out of range [:4] with capacity 0`
PFLog	60 B `00`	`index out of range [60] with length 60`
USB	41 B `00`	`index out of range [1] with length 1`
USBRequestBlockSetup	0 B / 2 B	`index out of range [0]` / `[:4] with capacity 2`
Prism	0 B	`slice bounds out of range [:4] with capacity 0`
FDDI	0 B	`slice bounds out of range [:7] with capacity 0`
AGUEVar0	1 B `00` / 4 B `ff`	`index out of range [1]` / `[:35] with capacity 4`
AGUEVar1	1 B `00` / 4 B `ff`	`index out of range [1]` / `[:35] with capacity 4`

ARP is representative of the arithmetic ones:

arpLength := 8 + 2*arp.HwAddressSize + 2*arp.ProtAddressSize   // all uint8 -- wraps
if len(data) < int(arpLength) { ... }                          // guard passes
arp.SourceHwAddress = data[8 : 8+arp.HwAddressSize]            // still uses the unwrapped value

HwAddressSize = ProtAddressSize = 0xff gives arpLength = 260 mod 256 = 4, the guard passes on an 8-byte buffer, and the next line slices data[8:263 mod 256] = data[8:7].

Reachability

Every one is reachable from a single ordinary Ethernet frame a rival can put on the wire — no handshake, no listener needed, the capturer parses it regardless:

  ARP        ethertype 0806   22 B frame
  PPPoE      ethertype 8864   20 B frame
  MPLS       ethertype 8847   15 B frame
  CTP        ethertype 9000   15 B frame
  EtherIP    ip proto 97      35 B frame
  IPSecESP   ip proto 50      35 B frame
  UDPLite    ip proto 136     35 B frame
  RUDP       ip proto 27      35 B frame
  GRE        ip proto 47      36 B frame
  IPSecAH    ip proto 51      46 B frame
  SCTP       ip proto 132     47 B frame
  Geneve     udp 6081         49 B frame
  GTPv1U     udp 2152         51 B frame
  SFlow      udp 6343         66 B frame

Linux SLL matters specifically for us: tcpdump -i any produces LINKTYPE_LINUX_SLL, so if the capturer ever runs on any rather than a named interface, a 16-byte frame reaches it. RadioTap, Prism, PFLog, USB and FDDI are link-type dependent and not reachable in our deployment.

Severity — what the recovery actually does

Both entry points recover by default, and I want to be precise about this because it is the difference between medium and critical:

gopacket.NewPacket recovers unless DecodeOptions{SkipDecodeRecovery: true}.
DecodingLayerParser.DecodeLayers recovers too — parser.go:304 installs defer panicToError(&err) unless DecodingLayerParserOptions{IgnorePanic: true}.

So a default caller gets an error, not a crash:

benign ARP  (42 B) -> decoded=[Ethernet ARP] err=<nil>
hostile ARP (22 B) -> decoded=[Ethernet] err=panic: runtime error: slice bounds out of range [8:7]

with IgnorePanic = true (documented as the low-latency setting):
  PANIC escapes DecodeLayers: runtime error: slice bounds out of range [8:7]

That leaves three real consequences:

Hard crash for opt-out callers. Both opt-outs are performance features that gopacket's own documentation encourages — IgnorePanic's doc says handling panics "does add latency to the process of decoding layers". Any consumer that took that advice has a remote crash from a 22-byte frame.
Throughput cost. Measured on this box:
```
benign  42 B frame:  38ns/packet = 26,315,789 packets/sec/core
hostile 22 B frame: 556ns/packet =  1,798,561 packets/sec/core   (15x slower)
```
15× is real but it still takes ~316 Mbit/s of these frames to saturate a core, so on its own this is a weak DoS. Worth knowing, not worth panicking about.
The packet is dropped from analysis. The layer never decodes, so whatever it carried is invisible. This is the part that matters for a monitoring tool, and it is free for the attacker.

Fix

Individually these are one-line guards. The more useful fix is structural — none of this class would have survived a test that tries short inputs, and there isn't one. Suggested addition to layers/:

// TestShortInputs asserts that no registered decoder panics on a truncated or
// malformed buffer, whatever the caller's recovery settings.
func TestShortInputs(t *testing.T) {
	pats := []byte{0x00, 0xff, 0x80, 0x0f, 0x3f}
	for i := 0; i < 2000; i++ {
		lt := gopacket.LayerType(i)
		if strings.HasPrefix(lt.String(), "UnknownLayerType") {
			continue
		}
		for n := 0; n <= 80; n++ {
			for _, p := range pats {
				buf := bytes.Repeat([]byte{p}, n)
				func() {
					defer func() {
						if r := recover(); r != nil {
							t.Errorf("%v panicked on %d bytes of 0x%02x: %v", lt, n, p, r)
						}
					}()
					pkt := gopacket.NewPacket(buf, lt,
						gopacket.DecodeOptions{SkipDecodeRecovery: true})
					for _, l := range pkt.Layers() {
						_ = l.LayerContents()
						_ = l.LayerPayload()
					}
				}()
			}
		}
	}
}

It runs in well under a minute, is fully deterministic, and would have caught all 39.

Two general rules the individual fixes should follow:

Compute lengths in int, not in the header field's own type. ARP, Geneve (#9) and Linux SLL all wrap because the arithmetic stays in uint8/uint16.
Guard against the largest index the function will actually use, not against the minimum header size. Several of these check the header size and then index into a variable-length area beyond it.

Verified against b7d9dbd on Go 1.24.4. PoCs: sweep, reach, dlp. Geneve is filed separately as #8/#9 because it has an additional evasion primitive.

**Severity: medium** · `layers/` — 26 distinct decoders, see table Not a single bug: a **class** of bug, found by a deterministic sweep rather than by fuzzing. Every one of these decoders validates a minimum length and then indexes past it, or derives an offset with arithmetic that wraps. ## Method For every registered `gopacket.LayerType`, feed buffers of every length 0–80 filled with each of six byte patterns (`00`, `ff`, `inc`, `80`, `0f`, `3f`), with `SkipDecodeRecovery: true` so panics surface, then call `Layers()`, `LayerContents()` and `LayerPayload()` on the result. ``` swept 2000 registered layer types x 81 lengths x 6 patterns = 972000 probes layers with at least one panic: 26 distinct (layer, panic shape) pairs: 39 ``` The whole harness is about 60 lines and runs in under a minute. It is attached below as a suggested CI test. ## Results | layer | smallest trigger | panic | |---|---|---| | ARP | 8 B `ff` | `slice bounds out of range [8:7]` | | ARP | 8 B `80` | `slice bounds out of range [:136] with capacity 8` | | GRE | 0 B | `index out of range [0] with length 0` | | GRE | 2 B `00` | `slice bounds out of range [:4] with capacity 2` | | MPLS | 0 B | `slice bounds out of range [:4] with capacity 0` | | PPPoE | 0 B / 6 B `ff` | `[:4] with capacity 0` / `[6:5]` | | EtherIP | 0 B / 1 B | `index out of range [0]` / `[:2] with capacity 1` | | IPSecAH | 12 B `00` | `slice bounds out of range [12:8]` | | IPSecESP | 0 B | `slice bounds out of range [:4] with capacity 0` | | UDPLite | 0 B | `slice bounds out of range [:2] with capacity 0` | | SCTP | 13 B `00` | `slice bounds out of range [:4] with capacity 1` | | RUDP | 0 B | `slice bounds out of range [:6] with capacity 0` | | GTPv1U | 9 B `00` | `index out of range [1] with length 1` | | Geneve | 7 B `00` | `slice bounds out of range [:8] with capacity 7` (see #8) | | ERSPAN Type II | 0 B / 1 B | `index out of range [0]` / `[:2] with capacity 1` | | EthernetCTP | 0 B | `slice bounds out of range [:2] with capacity 0` | | CiscoDiscovery | 0 B | `slice bounds out of range [:4] with capacity 0` | | Linux SLL | 16 B `ff` | `slice bounds out of range [6:5]` | | Linux SLL | 16 B `inc` | `slice bounds out of range [:1035] with capacity 16` | | RadioTap | 8 B `inc` | `index out of range [8] with length 8` | | RadioTap | 14 B `inc` | `slice bounds out of range [770:14]` | | SFlow | 0 B | `slice bounds out of range [4:0]` | | SFlow | 24 B `ff` | `slice bounds out of range [:4] with capacity 0` | | PFLog | 60 B `00` | `index out of range [60] with length 60` | | USB | 41 B `00` | `index out of range [1] with length 1` | | USBRequestBlockSetup | 0 B / 2 B | `index out of range [0]` / `[:4] with capacity 2` | | Prism | 0 B | `slice bounds out of range [:4] with capacity 0` | | FDDI | 0 B | `slice bounds out of range [:7] with capacity 0` | | AGUEVar0 | 1 B `00` / 4 B `ff` | `index out of range [1]` / `[:35] with capacity 4` | | AGUEVar1 | 1 B `00` / 4 B `ff` | `index out of range [1]` / `[:35] with capacity 4` | ARP is representative of the arithmetic ones: ```go arpLength := 8 + 2*arp.HwAddressSize + 2*arp.ProtAddressSize // all uint8 -- wraps if len(data) < int(arpLength) { ... } // guard passes arp.SourceHwAddress = data[8 : 8+arp.HwAddressSize] // still uses the unwrapped value ``` `HwAddressSize = ProtAddressSize = 0xff` gives `arpLength = 260 mod 256 = 4`, the guard passes on an 8-byte buffer, and the next line slices `data[8:263 mod 256] = data[8:7]`. ## Reachability Every one is reachable from a single ordinary Ethernet frame a rival can put on the wire — no handshake, no listener needed, the capturer parses it regardless: ``` ARP ethertype 0806 22 B frame PPPoE ethertype 8864 20 B frame MPLS ethertype 8847 15 B frame CTP ethertype 9000 15 B frame EtherIP ip proto 97 35 B frame IPSecESP ip proto 50 35 B frame UDPLite ip proto 136 35 B frame RUDP ip proto 27 35 B frame GRE ip proto 47 36 B frame IPSecAH ip proto 51 46 B frame SCTP ip proto 132 47 B frame Geneve udp 6081 49 B frame GTPv1U udp 2152 51 B frame SFlow udp 6343 66 B frame ``` `Linux SLL` matters specifically for us: `tcpdump -i any` produces `LINKTYPE_LINUX_SLL`, so if the capturer ever runs on `any` rather than a named interface, a 16-byte frame reaches it. `RadioTap`, `Prism`, `PFLog`, `USB` and `FDDI` are link-type dependent and not reachable in our deployment. ## Severity — what the recovery actually does Both entry points recover by default, and I want to be precise about this because it is the difference between medium and critical: - `gopacket.NewPacket` recovers unless `DecodeOptions{SkipDecodeRecovery: true}`. - `DecodingLayerParser.DecodeLayers` recovers too — `parser.go:304` installs `defer panicToError(&err)` unless `DecodingLayerParserOptions{IgnorePanic: true}`. So a default caller gets an error, not a crash: ``` benign ARP (42 B) -> decoded=[Ethernet ARP] err=<nil> hostile ARP (22 B) -> decoded=[Ethernet] err=panic: runtime error: slice bounds out of range [8:7] with IgnorePanic = true (documented as the low-latency setting): PANIC escapes DecodeLayers: runtime error: slice bounds out of range [8:7] ``` That leaves three real consequences: 1. **Hard crash for opt-out callers.** Both opt-outs are performance features that gopacket's own documentation encourages — `IgnorePanic`'s doc says handling panics "does add latency to the process of decoding layers". Any consumer that took that advice has a remote crash from a 22-byte frame. 2. **Throughput cost.** Measured on this box: ``` benign 42 B frame: 38ns/packet = 26,315,789 packets/sec/core hostile 22 B frame: 556ns/packet = 1,798,561 packets/sec/core (15x slower) ``` 15× is real but it still takes ~316 Mbit/s of these frames to saturate a core, so on its own this is a weak DoS. Worth knowing, not worth panicking about. 3. **The packet is dropped from analysis.** The layer never decodes, so whatever it carried is invisible. This is the part that matters for a monitoring tool, and it is free for the attacker. ## Fix Individually these are one-line guards. The more useful fix is structural — none of this class would have survived a test that tries short inputs, and there isn't one. Suggested addition to `layers/`: ```go // TestShortInputs asserts that no registered decoder panics on a truncated or // malformed buffer, whatever the caller's recovery settings. func TestShortInputs(t *testing.T) { pats := []byte{0x00, 0xff, 0x80, 0x0f, 0x3f} for i := 0; i < 2000; i++ { lt := gopacket.LayerType(i) if strings.HasPrefix(lt.String(), "UnknownLayerType") { continue } for n := 0; n <= 80; n++ { for _, p := range pats { buf := bytes.Repeat([]byte{p}, n) func() { defer func() { if r := recover(); r != nil { t.Errorf("%v panicked on %d bytes of 0x%02x: %v", lt, n, p, r) } }() pkt := gopacket.NewPacket(buf, lt, gopacket.DecodeOptions{SkipDecodeRecovery: true}) for _, l := range pkt.Layers() { _ = l.LayerContents() _ = l.LayerPayload() } }() } } } } ``` It runs in well under a minute, is fully deterministic, and would have caught all 39. Two general rules the individual fixes should follow: - **Compute lengths in `int`, not in the header field's own type.** ARP, Geneve (#9) and Linux SLL all wrap because the arithmetic stays in `uint8`/`uint16`. - **Guard against the largest index the function will actually use**, not against the minimum header size. Several of these check the header size and then index into a variable-length area beyond it. --- *Verified against `b7d9dbd` on Go 1.24.4. PoCs: `sweep`, `reach`, `dlp`. Geneve is filed separately as #8/#9 because it has an additional evasion primitive.*

claude added the

labels

2026-08-26 10:02:41 +00:00

noi commented

2026-08-26 10:03:24 +00:00

Owner

Can this be exploited? Is there a way to reproduce this issue over the wire? Like someone would send the malicious packets to a service that's being dumped, then Daisy would parse it with gopacket. If not, close the issue.

claude commented

2026-08-26 14:30:59 +00:00

Author

Collaborator

Fixed on fix/issue-11-layer-short-input, all 26 decoders, verified against the full 972,000-probe sweep (zero panics, down from 26 layers / 39 crash shapes) plus the existing test suite.

Two things surfaced while writing the fix that go beyond what the original short-input sweep found, both real and both fixed alongside:

RadioTap had no bounds checking at all, not just an off-by-one. Every field is read at an offset driven entirely by which wire-controlled Present bits are set, with zero length checks anywhere in DecodeFromBytes. Rewrote it field-by-field with a need(n) guard before every read, bounded the extended-presence-bitmap loop (it used to read data[offset:offset+4] unconditionally while chasing extension words), and checked the it_len-derived payload slice and the Datapad realignment against the actual buffer.

SCTP's SACK gap-ACK/dup-TSN loop had a real unbounded read, independent of the short-input class (it needs a valid header, not a truncated one, so the sweep didn't catch it): the code pre-computed a clamped capacity for make() with a comment explaining it was deliberately capped "so we're not allocating tons of memory," but the loop bound used the raw wire-controlled NumGapACKs/NumDuplicateTSNs fields directly, not the clamped value. A SACK chunk with a large declared count and a short actual buffer reads straight past the end. Fixed by clamping the loop bound itself, not just the initial capacity.

Also worth a note for anyone extending USB: fixed a live unsigned-underflow in the Data-flagged payload slice (len(data) - UrbDataLength wraps when the declared length exceeds the buffer) that the uniform-byte sweep can't reach — it needs data[14]!=0 && data[15]==0, which no single repeated byte value produces — but is real and one crafted packet away.

One behavioral note for review: decodeString in sflow.go (used by decodePortnameCounters) gained an error return, since it previously had no way to reject a string length that exceeded what remained in the buffer. Its one caller is updated; no other change to its return semantics (it still returns the padded length as the first value, matching the original — I initially got that wrong and a existing test caught it).

PoC/verification: sweep and reach on branch pentest/2026-08-poc, re-run against the fix — 0/972000 panics.

Fixed on `fix/issue-11-layer-short-input`, all 26 decoders, verified against the full 972,000-probe sweep (zero panics, down from 26 layers / 39 crash shapes) plus the existing test suite. Two things surfaced while writing the fix that go beyond what the original short-input sweep found, both real and both fixed alongside: **RadioTap had no bounds checking at all**, not just an off-by-one. Every field is read at an offset driven entirely by which wire-controlled `Present` bits are set, with zero length checks anywhere in `DecodeFromBytes`. Rewrote it field-by-field with a `need(n)` guard before every read, bounded the extended-presence-bitmap loop (it used to read `data[offset:offset+4]` unconditionally while chasing extension words), and checked the `it_len`-derived payload slice and the Datapad realignment against the actual buffer. **SCTP's SACK gap-ACK/dup-TSN loop had a real unbounded read**, independent of the short-input class (it needs a valid header, not a truncated one, so the sweep didn't catch it): the code pre-computed a clamped capacity for `make()` with a comment explaining it was deliberately capped "so we're not allocating tons of memory," but the **loop bound** used the raw wire-controlled `NumGapACKs`/`NumDuplicateTSNs` fields directly, not the clamped value. A SACK chunk with a large declared count and a short actual buffer reads straight past the end. Fixed by clamping the loop bound itself, not just the initial capacity. Also worth a note for anyone extending `USB`: fixed a live unsigned-underflow in the `Data`-flagged payload slice (`len(data) - UrbDataLength` wraps when the declared length exceeds the buffer) that the uniform-byte sweep can't reach — it needs `data[14]!=0 && data[15]==0`, which no single repeated byte value produces — but is real and one crafted packet away. One behavioral note for review: `decodeString` in `sflow.go` (used by `decodePortnameCounters`) gained an error return, since it previously had no way to reject a string length that exceeded what remained in the buffer. Its one caller is updated; no other change to its return semantics (it still returns the *padded* length as the first value, matching the original — I initially got that wrong and a existing test caught it). *PoC/verification: `sweep` and `reach` on branch `pentest/2026-08-poc`, re-run against the fix — 0/972000 panics.*