Suricata

Reported by Communications Security Establishment (CSE):

## Summary
The offline ERF capture reader (`--erf-in`) trusts the 16-bit `rlen` field taken directly from the ERF file header and uses it as the byte count for an `fread()` straight into the trailing flexible-array buffer of a pooled `Packet` structure. The only check applied is a *lower* bound (`rlen >= sizeof(DagRecord)`); no upper bound is enforced against the `default_packet_size` (1514 bytes in this run mode). A crafted ERF file with `rlen = 0xFFFF` therefore writes ~65 KB of fully attacker-controlled data past the end of a 1514-byte heap buffer, corrupting adjacent pooled `Packet` objects (which contain function pointers) or unrelated heap chunks. The result is a reliable crash and a strong primitive toward code execution in the Suricata process, gated on the operator feeding Suricata a hostile or attacker-influenced ERF file.

## Affected Piece of Code
- **File:** `src/source-erf-file.c`
- **Function / Location:** `ReadErfRecord()` ~L154-L210 (specifically L171-L178)
- **Subsystem:** capture — Capture sources (af-packet, af-xdp, dpdk, netmap, nfq, pcap, etc.) and packet queue

```c
/* src/source-erf-file.c */
161:    size_t r = fread(&dr, sizeof(DagRecord), 1, etv->erf);
...
171:    uint16_t rlen = SCNtohs(dr.rlen);
172:    uint16_t wlen = SCNtohs(dr.wlen);
173:    if (rlen < sizeof(DagRecord)) {
174:        SCLogError("Bad ERF record, " 
175:                   "record length less than size of header");
176:        SCReturnInt(TM_ECODE_FAILED);
177:    }
178:    r = fread(GET_PKT_DATA(p), rlen - sizeof(DagRecord), 1, etv->erf);
179:    if (r < 1) {
180:        if (feof(etv->erf)) {
181:            SCLogInfo("End of ERF file reached");
182:        }
183:        else {
184:            SCLogInfo("Error reading ERF record");
185:        }
186:        SCReturnInt(TM_ECODE_FAILED);
187:    }

/* src/decode.h */
210:#define GET_PKT_DATA(p)            (((p)->ext_pkt == NULL) ? GET_PKT_DIRECT_DATA(p) : (p)->ext_pkt)
211:#define GET_PKT_DIRECT_DATA(p)     (p)->pkt_data
701:    uint8_t pkt_data[];
711:#define DEFAULT_PACKET_SIZE (DEFAULT_MTU + ETHERNET_HEADER_LEN)
715:#define SIZE_OF_PACKET (default_packet_size + sizeof(Packet))
```

## The Bug
`ReadErfRecord()` parses one record from an Endace ERF capture file. It first reads the fixed 18-byte packed `DagRecord` header from the file (L161), byte-swaps the big-endian `rlen` field (L171), and then issues a second `fread()` of `rlen - sizeof(DagRecord)` bytes directly into `GET_PKT_DATA(p)` (L178). For a freshly acquired packet `p->ext_pkt` is `NULL`, so `GET_PKT_DATA(p)` resolves to `p->pkt_data` (`decode.h:210-211`) — the trailing C99 flexible array at the very end of the heap-allocated `Packet` structure (`decode.h:701`). The capacity of that array is exactly `default_packet_size` bytes, fixed at allocation time (`SIZE_OF_PACKET = default_packet_size + sizeof(Packet)`, `decode.h:715`).

The only validation applied to `rlen` is the lower-bound check at L173 (`rlen < sizeof(DagRecord)`), which guards against underflow of the subtraction. There is **no upper-bound check** against `default_packet_size`, `MAX_PAYLOAD_SIZE`, or anything else. Because `rlen` is a `uint16_t` taken verbatim from the input file, an attacker can set it to `0xFFFF`, causing the second `fread()` to copy `65535 - 18 = 65517` bytes of attacker-supplied file content into a buffer that is only 1514 bytes long — an out-of-bounds heap write of roughly 64003 bytes with fully controlled contents.

**Call chain / reachability.** The entry point is the `--erf-in <file>` command-line option used for offline analysis of ERF captures:

1. `src/suricata.c:1763-1767` — `ParseCommandLine()` handles `--erf-in`, sets `suri->run_mode = RUNMODE_ERF_FILE`, and stores the path under conf key `erf-file.file`.
2. `src/suricata.c:2604-2662` — `PostConfLoadedSetup()` computes `default_packet_size`. `RUNMODE_ERF_FILE` is not special-cased and falls through to the `default:` branch, which (absent a `default-packet-size` YAML override) sets `default_packet_size = DEFAULT_PACKET_SIZE = DEFAULT_MTU (1500) + ETHERNET_HEADER_LEN (14) = 1514`. Every pooled `Packet` is therefore allocated as `sizeof(Packet) + 1514` bytes with a 1514-byte trailing `pkt_data[]`.
3. `RunModeDispatch()` → `RunModeErfFileAutoFp()` / `RunModeErfFileSingle()` (`src/runmode-erf-file.c`) creates a `pktacqloop` thread bound to `TMM_RECEIVEERFFILE`.
4. The thread executes `TmThreadsSlotPktAcqLoop` → `ReceiveErfFileThreadInit()` (`src/source-erf-file.c:216`, which `fopen()`s the attacker-supplied file) → `ReceiveErfFileLoop()` (L112).
5. Each loop iteration calls `PacketGetFromQueueOrAlloc()` (L132), which returns a fresh `Packet` with `ext_pkt == NULL`, and then invokes `ReadErfRecord()` (L154).
6. `ReadErfRecord()` reads the 18-byte header, derives `rlen`, performs only the lower-bound check, and executes `fread(GET_PKT_DATA(p), rlen - sizeof(DagRecord), 1, etv->erf)` at L178 — writing up to 65517 attacker bytes into the 1514-byte `p->pkt_data[]` flex array.

Because pooled `Packet` objects are allocated back-to-back from the packet pool, the overflow lands in the *next* `Packet` structure(s) in memory (and/or adjacent unrelated heap chunks). `Packet` contains numerous function pointers — `ReleasePacket`, `BypassPacketsFlow`, `livedev`, `flow` — and linked-list pointers, all of which become attacker-controlled after the overflow.

For comparison, the sibling *live* DAG capture source handles the identical situation safely: `src/source-erf-dag.c:503` copies the record body via `PacketCopyData()`, which checks `default_packet_size` and transparently spills oversize payloads to a dynamically allocated `ext_pkt`. The file-based reader simply omitted this safeguard.

A secondary, related defect is that L195 sets `GET_PKT_LEN(p) = wlen` using the file-supplied `wlen` without bounding it by the number of bytes actually read, but that is overshadowed by the direct write overflow above.

**Vulnerability class:** heap-buffer-overflow (CWE-122).

## Reproduction Results
*The following trigger is analytically derived from source review; it has not been executed under ASAN as part of this audit, but every step is deterministic and requires no race or unusual configuration.*

1. **Create a malicious ERF file** `evil.erf` consisting of one 18-byte `DagRecord` header followed by 65517 payload bytes. The packed header layout is:

   | Offset | Field        | Bytes                     | Notes                                                       |
   |-------:|--------------|---------------------------|-------------------------------------------------------------|
   | 0..7   | `ts`         | `00 00 00 00 00 00 00 00` | timestamp, ignored                                          |
   | 8      | `type`       | `02`                      | `DAG_TYPE_ETH` — required so the record passes the L190 check |
   | 9      | `flags`      | `00`                      |                                                             |
   | 10..11 | `rlen` (BE)  | `FF FF`                   | 65535                                                       |
   | 12..13 | `lctr`       | `00 00`                   |                                                             |
   | 14..15 | `wlen` (BE)  | `05 EA`                   | 1514 (value irrelevant to the overflow)                     |
   | 16..17 | `pad`        | `00 00`                   |                                                             |

   Then append `0xFFFF - 18 = 65517` bytes of `0x41`. One-liner:

   ```sh
   python3 -c "import struct,sys; sys.stdout.buffer.write(b'\x00'*8 + b'\x02' + b'\x00' + struct.pack('>H',0xFFFF) + b'\x00\x00' + struct.pack('>H',1514) + b'\x00\x00' + b'A'*65517)" > evil.erf
   ```

2. **Use a stock configuration.** Ensure `suricata.yaml` does **not** set `default-packet-size` (or sets it ≤ 65517). The shipped default leaves it unset, so `default_packet_size` becomes 1514 for this run mode.

3. **Run Suricata on the file** (rules are irrelevant; `-S /dev/null` is fine):

   ```sh
   suricata -c suricata.yaml --erf-in evil.erf -l /tmp -S /dev/null
   ```

4. **Result.** The receive thread enters `ReceiveErfFileLoop()` → `ReadErfRecord()`. The second `fread()` at `src/source-erf-file.c:178` writes 65517 bytes into the 1514-byte `p->pkt_data[]` flex array.
   - Under AddressSanitizer: immediate `heap-buffer-overflow WRITE of size 65517` report pointing at `src/source-erf-file.c:178`.
   - Without ASAN: adjacent pooled `Packet` structures and/or glibc heap metadata are corrupted; the process typically crashes shortly afterward when the corrupted neighbour `Packet` is dequeued (its overwritten `ReleasePacket` / list pointers are dereferenced) or when malloc detects metadata corruption.

## Severity
**MEDIUM.** This is a heap buffer overflow that writes up to ~64 KB of fully attacker-controlled bytes past the end of a pooled `Packet` object. The packet pool stores many `SIZE_OF_PACKET`-sized allocations contiguously, and `Packet` carries multiple function pointers (`ReleasePacket`, `BypassPacketsFlow`), device/flow pointers, and intrusive list links — corrupting the next `Packet` in the pool therefore yields a strong primitive toward arbitrary code execution in the Suricata process (which is frequently launched as root before privilege drop). At minimum it is a reliable denial of service of the IDS.

The rating is held at MEDIUM rather than HIGH because exploitation requires the operator to invoke `suricata --erf-in` on an attacker-supplied or attacker-influenced ERF capture file. This is the same threat model as feeding a hostile pcap to an analyst workstation or an automated capture-replay/regression pipeline; it is **not** reachable from live network traffic against a passively listening sensor.

## Suggested Fix
Do not `fread()` directly into the Packet's inline flex-array buffer. Mirror the approach already used by the live DAG source (`src/source-erf-dag.c:503`): copy the record body via `PacketCopyData()` / `PacketSetData()`, which checks `default_packet_size` and transparently allocates `ext_pkt` for oversize payloads. Additionally, hard-cap `rlen` against `MAX_PAYLOAD_SIZE`. Minimal patch:

```diff
--- a/src/source-erf-file.c
+++ b/src/source-erf-file.c
@@ -170,12 +170,21 @@ static inline TmEcode ReadErfRecord(ThreadVars *tv, Packet *p, void *data)
     }
     uint16_t rlen = SCNtohs(dr.rlen);
     uint16_t wlen = SCNtohs(dr.wlen);
     if (rlen < sizeof(DagRecord)) {
         SCLogError("Bad ERF record, " 
                    "record length less than size of header");
         SCReturnInt(TM_ECODE_FAILED);
     }
-    r = fread(GET_PKT_DATA(p), rlen - sizeof(DagRecord), 1, etv->erf);
+    uint32_t pktlen = rlen - sizeof(DagRecord);
+    if (pktlen > MAX_PAYLOAD_SIZE) {
+        SCLogError("Bad ERF record, record length %u exceeds max %u", pktlen,
+                MAX_PAYLOAD_SIZE);
+        SCReturnInt(TM_ECODE_FAILED);
+    }
+    if (PacketCallocExtPkt(p, pktlen) < 0)  /* or read into stack buf then PacketCopyData */
+        SCReturnInt(TM_ECODE_FAILED);
+    r = fread(GET_PKT_DATA(p), pktlen, 1, etv->erf);
```

A simpler defensive variant: read the body into a local `uint8_t buf[65536]`, then call `PacketCopyData(p, buf, pktlen)` (which internally checks `default_packet_size` and falls back to `ext_pkt`), and set `GET_PKT_LEN(p) = pktlen`. This also fixes the separate issue that `wlen` is currently assigned to `GET_PKT_LEN(p)` without being bounded by the number of bytes actually read.