Actions
Bug #9116
open
LS
VJ
detect: memory leaks in the alert output
Bug #9116:
detect: memory leaks in the alert output
Affected Versions:
Effort:
Difficulty:
Label:
Description
Aggregate ticket for multiple memory leak bug reports relevant in the same/similar areas.
# D4 — alert-json-info-leak-suppressed ## Finding - **File/line:** `src/detect-engine-alert.c`, `PacketAlertSetContext()` (lines 327–371), reached from `PacketAlertSet()` (line 386) → `AlertQueueAppend()` (line 412) - **Bug class:** heap memory leak (per-alert, unbounded across packets) `PacketAlertSetContext()` attaches JSON context to every alert enqueued in the per-thread alert queue (`AlertQueueAppend`): for each `pcre` `alert:` capture belonging to the matching signature it allocates a `struct PacketContextData` (`SCCalloc`) and `SCStrdup`s the captured JSON string into `pa->json_info` (line 363). Those allocations are only released for alerts that are actually copied into the packet's final alert array: `PacketAlertRecycle()` (`src/decode.c:152`) frees `json_info` for `p->alerts.alerts[0..cnt)`. `PacketAlertFinalizeProcessQueue()` (`src/detect-engine-alert.c:~676`) walks the queue and drops entries without copying them in several cases: - **threshold suppression** — `PacketAlertHandle()` returns 0/2 (`p->alerts.suppressed++`) - discarded on queue/packet-alert overflow (`p->alerts.discarded++`) - firewall skip (`skip_td`/`skip_fw`) paths - pass-action break-outs that stop before copying - non-alert actions (`(s->action & (ACTION_ALERT|ACTION_PASS)) == 0`) In every one of those cases, the `json_info` chain allocated when the alert was appended is simply abandoned when the queue slot is reset by the next packet (`det_ctx->alert_queue_size = 0` in `src/detect.c:1017` never frees the structs). Traffic that keeps matching a pcre-capture rule while its alerts are suppressed therefore leaks `sizeof(PacketContextData) + strlen(capture) + 1` bytes per matching transaction, indefinitely. ## How it reproduces 1. A rule uses the pcre **alert-var capture** syntax — after the closing `/` of the regex, a comma-separated variable list with the `alert:` prefix: `pcre:"/^([a-z0-9.\-]+)/,alert:host_captured";` (documented in `doc/userguide/rules/payload-keywords.rst`, "PCRE extraction"). Each match calls `DetectAlertStoreMatch()` (`src/detect-pcre.c:161`) which stashes the capture in `det_ctx->json_content[]`, and `PacketAlertSetContext()` then duplicates it into the queued alert. 2. The rule carries `threshold: type limit, track by_flow, count 1, seconds 3600;` — one alert fires, all further matches on the flow are suppressed. 3. The pcap is one established TCP flow with **200 HTTP requests** (distinct `Host:` values so the capture differs each time). 100 requests are parsed before the flow times out / is re-inspected at flow end; 1 alert fires, 99 are suppressed — and those 99 queue entries' `json_info` are never freed. ## Notes / caveats - Suricata's own exit code is 1 (LSAN failure) — that is the signal, not a crash. - Only 100 of the 200 requests show up in `alerts_suppressed` because HTTP tx inspection for this flow happens partly at flow-timeout reinspection; irrelevant to the leak. - The same leak affects the other listed paths (discarded, firewall-skip, pass-break, non-alert-action); threshold suppression is just the easiest to trigger deterministically.
# D5 — `p->alerts.drop` json_info memory leak (drop action + pcre `alert:` capture)
## Finding
- **File:** `src/detect-engine-alert.c`, `AlertQueueAppend()`, lines ~397-401
- **Bug class:** memory leak (missing free path)
- **Affected allocation:** `PacketAlertSetContext()` (same file, ~line 326-370) — allocates a
`struct PacketContextData` node (SCCalloc) and strdup's `json_string` for every pcre
`alert:<name>` capture group matched by a signature.
### The bug
`AlertQueueAppend()` stores an extra copy of the alert in `p->alerts.drop` the first time a
`drop`-action signature matches:
```c
if (p->alerts.drop.action == 0 && s->action & ACTION_DROP) {
p->alerts.drop = PacketAlertSet(det_ctx, s, tx_id, alert_flags); /* line 399 */
}
```
`PacketAlertSet()` calls `PacketAlertSetContext()`, which populates `pa->json_info` with a freshly
allocated linked list when the signature has pcre `alert:` captures.
The free sites only ever look at the *regular* alert array:
- `PacketAlertRecycle()` (`src/decode.c:152`) is called from `PACKET_RECYCLE`/`PacketRecycle`
(`src/packet.c:130-135`) **only on `p->alerts.alerts`**, and only after setting
`p->alerts.drop.action = 0` — the `json_info` chain hanging off `p->alerts.drop` is never freed
and its pointer is simply overwritten on the next drop.
- `PacketAlertFree()` (`src/decode.c:168`) frees the packet's alert array at pool teardown, not
`p->alerts.drop`.
Result: every packet that (a) matches a `drop` rule with pcre `alert:` captures and (b) is the
first drop match on that packet, leaks `N` nodes + `N` strings where N = number of capture
groups. In a long-running IPS deployment with drop rules that extract captures this is an
unbounded, per-dropped-packet heap leak.
## Expected signal
- LeakSanitizer reports `ERROR: LeakSanitizer: detected memory leaks` on the drop run.
- Leak stack goes through `PacketAlertSetContext <- PacketAlertSet <- AlertQueueAppend`
(`detect-engine-alert.c:399` — the `p->alerts.drop` assignment).
- The control run (alert action) reports **no** leak — the regular alert queue's `json_info` is
freed by `PacketAlertRecycle()`.
## Notes
- The leak is 62 bytes for a single matching packet with two captures; it scales per dropped
packet, so it is a slow but unbounded leak in production IPS mode.
- The alert is raised from the flow-timeout pseudo-packet path (`FlowWorkerFlowTimeout`) in this
pcap because the detection runs on reassembled stream data; the leak path is identical for
per-packet drop matches — what matters is that `AlertQueueAppend()` takes the drop branch.
- Suggested fix: free `p->alerts.drop.json_info` in `PacketRecycle()` (next to
`p->alerts.drop.action = 0`, `src/packet.c:130`) — e.g. add a `PacketAlertDropReset()` that
recycles the drop copy's `json_info` chain — or drop the `json_info` from the copy stored in
`p->alerts.drop` if the drop logger does not need it.
# D6 — alert json_info leak in the pass-without-alert branch of `PacketAlertFinalizeProcessQueue` > **Re-validated on 8.0.7 RELEASE (`d681600f3`, branch `release-8.0.7-260913122826`): still > REPRODUCED — this repo's history briefly marked it "FIXED-UPSTREAM" after testing the wrong > Suricata version (9.0.0-dev main, where fix commit `5b19508661` "detect/alert: fully add > pass-only rules to alert queue" lives). That commit is NOT an ancestor of this branch's HEAD > (`git merge-base --is-ancestor 5b19508661 HEAD` fails) — it was never backported to > `release-8.0.7-260913122826`. ## Finding - **File/line:** `src/detect-engine-alert.c`, `PacketAlertFinalizeProcessQueue()`, the pass branch at lines 727–750 (branch condition on line 732, the `break`/`continue` without `p->alerts.cnt++` at lines 734–739; the `cnt++` that gets skipped is line 740). Consequence lands in `src/packet.c:131-135` (`PacketReinit` → `PacketAlertRecycle`) which only frees json_info for indices `[0, p->alerts.cnt)`. - **Bug class:** memory leak (unreachable heap object), security-adjacent: unbounded memory growth on a long-running sensor, triggerable by ordinary network traffic matching a crafted rule. - **Allocations leaked:** `PacketContextData` (SCCalloc) + `json_string` (SCStrdup), both from `PacketAlertSetContext()` (`src/detect-engine-alert.c:327-371`). ## Mechanics 1. A rule with action `pass` and a pcre capture named `alert:<key>` matches a packet. `AlertQueueAppend()` → `PacketAlertSet()` → `PacketAlertSetContext()` allocates `json_info` for the captured group and attaches it to the queued `PacketAlert`. 2. In `PacketAlertFinalizeProcessQueue()` the alert passes thresholding, so it is stored: `p->alerts.alerts[p->alerts.cnt] = *pa;` (line 728). 3. The rule action is PASS without ALERT, so the branch at line 732 hits and the function `break`s (non-firewall mode) — **`p->alerts.cnt` is never incremented**. 4. When the pooled `Packet` is later recycled, `PacketReinit()` sees `cnt == 0` and never frees the json_info sitting at slot `[0]`. 5. The next packet reuses the pooled `Packet` and raises an alert at slot index 0, overwriting the `PacketAlert` (and the `json_info` pointer) with a fresh one. The old `json_info` becomes unreachable → leak. (At pool shutdown `PacketFree()` iterates all slots, so only slots overwritten *during* the run leak.) Each matching packet leaks one `PacketContextData` (~16 B) plus one duplicated string (capture-length dependent). ## Notes / caveats - `--runmode single` is essential. With the default autofp runmode the capture thread hands packets to a separate detect thread via a queue; enough packets are in flight that each pass-rule packet gets a fresh pooled `Packet` whose slot is never overwritten before exit, so `PacketFree()` cleans up and no leak is visible. In single mode one thread recycles the same pooled packet immediately, so every subsequent packet overwrites the leaked slot. - Suggested fix: increment `p->alerts.cnt` before the pass-branch `break`/ `continue` (or recycle the just-written slot before breaking), so the recycler sees the json_info-bearing slot. - Similar overwrite-without-recycle patterns exist for `p->alerts.drop` (`AlertQueueAppend`, set before queueing) — not covered here.
Actions