[packetbeat-parser-safety] Confirmed panics in DHCPv4 options, Memcache VALUE merge, and Cassandra EVENT inet parsing
- Dominant language
- Go
- Stars
- 12.7k
- Forks
- 5k
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 364
Description
## Findings
### 1. DHCPv4 option-length panic in `optionsToMap`
**Severity:** High
**Location:** `packetbeat/protos/dhcpv4/options.go:95-96`
**Evidence**
- Code reads an untrusted DHCP option payload with no length check:
- `if offset := dhcp.GetOneOption(dhcpv4.OptionTimeOffset); offset != nil {`
- `opts.Put("utc_time_offset_sec", int32(binary.BigEndian.Uint32(offset)))`
- `binary.BigEndian.Uint32` panics when `len(offset) < 4`.
- The option value comes from on-wire DHCP option bytes and can be malformed/truncated.
**What is wrong**
- Parsed packet bytes are used as a fixed-width read (`Uint32`) without validating payload length.
**Why it matters**
- A malformed DHCP packet with `OptionTimeOffset` shorter than 4 bytes can crash parser execution with `index out of range`.
**Suggested fix**
- Guard length before decoding, e.g. only decode when `len(offset) >= 4`; otherwise skip or return parse error.
**Reproducer test snippet**
```go
func TestOptionsToMap_ShortTimeOffset_NoPanic(t *testing.T) {
pkt := &dhcpv4.DHCPv4{Options: dhcpv4.Options{byte(dhcpv4.OptionTimeOffset): {0x01}}}
_, _ = optionsToMap(pkt)
}
```
### 2. Memcache VALUE merge panic from wrong slice cap source
**Severity:** High
**Location:** `packetbeat/protos/memcache/memcache.go:304-310`
**Evidence**
- In `mergeValueMessages`, `delta` is derived from remaining capacity, then capped against the wrong slice:
- `delta = mc.config.maxValues - len(prev.values)`
- `if delta > len(prev.values) { delta = len(prev.values) }`
- `prev.values = append(prev.values, msg.values[0:delta]...)`
- `delta` should be capped against `len(msg.values)`, not `len(prev.values)`.
**What is wrong**
- With `maxValues > 0`, `delta` can exceed `len(msg.values)`, producing `msg.values[0:delta]` out-of-range panic.
**Why it matters**
- A network peer can send multiple `VALUE` records; in a realistic merge path this can crash parsing when value-capture is enabled.
**Suggested fix**
- Replace the cap with `if delta > len(msg.values) { delta = len(msg.values) }`.
**Reproducer test snippet**
```go
func TestMergeValueMessages_DeltaCap_NoPanic(t *testing.T) {
mc := &memcache{config: parserConfig{maxValues: 5}}
prev := &message{command: &commandType{code: memcacheResValue}, values: []memcacheData\{\{[]byte("a")}, {[]byte("b")}}}
msg := &message{command: &commandType{code: memcacheResValue}, values: []memcacheData\{\{[]byte("c")}}}
_, _ = mergeValueMessages(mc, prev, msg)
}
```
### 3. Cassandra compressed EVENT `ReadInet` bounds panic
**Severity:** High
**Location:** `packetbeat/protos/cassandra/internal/gocql/array_decoder.go:171-177`
**Evidence**
- `ReadInet` validates `len(data) < 1` after reading size byte, then slices with `size`:
- `size := data[0]`
- `data = *f.Data`
- `if len(data) < 1 { ... }`
- `copy(ip, data[:size])`
- For `size == 4` or `16` with insufficient remaining bytes, `data[:size]` panics.
**What is wrong**
- Bounds check uses constant `1` instead of `int(size)` before slicing by `size`.
**Why it matters**
- Malformed compressed Cassandra EVENT frames can trigger a runtime panic (`slice bounds out of range`), and `ReadFrame` re-panics runtime errors (`packetbeat/protos/cassandra/internal/gocql/frame.go:170-179`).
**Suggested fix**
- Validate `len(data) >= int(size)` before `copy(ip, data[:size])`.
**Reproducer test snippet**
```go
func TestReadFrame_CompressedEventShortInet_NoPanic(t *testing.T) {
payload := []byte{0x00, 0x0d, 'S','T','A','T','U','S','_','C','H','A','N','G','E', 0x00, 0x02, 'U','P', 0x10, 0x01}
var buf streambuf.Buffer
_, _ = buf.Write([]byte{0})
f := NewFramer(&buf, stubCompressor{decoded: payload})
f.Header = &frameHeader{Flags: flagCompress, Op: opEvent, BodyLength: 1}
_, _ = f.ReadFrame()
}
```
## Suggested Actions
- [ ] Add bounds checks in the three locations above and return parse errors instead of panicking.
- [ ] Add regression tests for malformed/truncated packet payloads matching the snippets.
- [ ] Sweep other packet-derived slice operations in parser paths for the same pattern (`size`/`count` used as index without `len` guard).
## Coverage (inspected this run)
`amqp`, `applayer`, `cassandra`, `dhcpv4`, `dns`, `http`, `icmp`, `memcache`, `mongodb`, `mysql`, `nfs`, `pgsql`, `redis`, `sip`, `tcp`, `thrift`, `tls`, `udp`, plus parser plumbing in `packetbeat/protos/protos.go` and `packetbeat/protos/registry.go`.
---
[What is this?](https://ela.st/github-ai-tools) | [From workflow: Sweeper: Packetbeat Parser Bounds Safety](https://github.com/elastic/beats/actions/runs/26396265888)
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
> - [x] expires on Jun 1, 2026, 10:50 AM UTC
Contributor guide
Assessment
This issue has not been assessed yet.