Project

General

Profile

Actions

Bug #9115

open
LS

firewall: null pointer dereference on packets whose direction has no sig group

Bug #9115: firewall: null pointer dereference on packets whose direction has no sig group

Added by Lukas Sismis about 16 hours ago.

Status:
New
Priority:
Normal
Assignee:
-
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

# D3 — frames-null-sgh-deref

## Finding

- **File/line:** `src/detect.c:2695` (function `DetectRunFrames`, starting line 2657)
- **Bug class:** NULL-pointer dereference (crash / potential DoS)
- **Code:** line 2660 reads `const SigGroupHead *const sgh = scratch->sgh;` — which can
  legitimately be NULL — and line 2695 dereferences it unconditionally:
  `if (sgh->frame_engines)`.
- **Contrast with the tx path:** the same condition is guarded in `DetectRunTx` at
  `src/detect.c:2328`: `if (sgh && sgh->tx_engines)`. The frame path simply misses the check.

## How it reproduces

`scratch->sgh` becomes NULL on a packet whose direction has no sig group. Sig groups are
built per direction (`RulesGroupByPorts(de_ctx, IPPROTO_TCP, SIG_FLAG_TOSERVER|TOCLIENT)`
in `src/detect-engine-build.c:1880-1883`), so a ruleset whose rules are all toserver-only
leaves `de_ctx->flow_gh[0]` (toclient) empty. Verified in debug logs:

```
Debug: detect: TCP list (nil), port 22, direction toclient, sghport (nil), sgh (nil)
Debug: detect: sgh = pflow->sgh_toclient; => (nil)
```

Three conditions must line up (all encoded in `reproducer.sh`):

1. **Firewall mode** (`--firewall`). In IDS mode `DetectRun()` bails out at
   `src/detect.c:129` when `scratch.sgh == NULL`, so the buggy code is unreachable.
   Firewall mode continues instead ("no sgh, need to apply default policies").
2. **A lopsided ruleset** — a single firewall rule with `hook:ssh:request_started`
   (a toserver app hook, so `SIG_FLAG_TOSERVER` only) using the `frame:ssh.record.pdu`
   keyword. The `frame:` keyword matters twice over: it makes the sig use frame
   inspection, and it calls `FrameConfigEnable()` (`src/detect-frame.c:150`) so the SSH
   parser actually creates frames — including in the toclient direction where no rule
   exists (`frame_config` is otherwise all-zero after `FrameConfigInit()`).
3. **Accepting default policies** (`firewall.policies.packet.filter: [accept:hook]`,
   `firewall.policies.app.default-policy: [accept:hook]`). The built-in packet:filter
   default is `drop:packet` (`detect-parse.c:4175`), and a drop short-circuits
   `DetectRun()` at line 153 before `DetectRunFrames()` is ever called.

Traffic: the SSH session pcap from `../suricata-verify/tests/ssh-newkeys/input.pcap`
(SSH server flight = toclient packets with frames, at pcap_cnt 7+).

## Expected signal

`SIGSEGV` (exit 139) in `DetectRunFrames` under the ASAN build, with Suricata's signal
handler printing the stack:

```
Error: suricata: stacktrace:sig 11:DetectRunFrames+0x00000297;DetectRun+0x0000107c;Detect+0x0000034f;FlowWorker+0x00000943;...
```

No data to display

Actions

Also available in: PDF Atom