Suricata

Reported by Communications Security Establishment (CSE):

## Summary

Suricata's crash-time stack-trace handler `SignalHandlerUnexpected()` in `src/suricata.c` formats each libunwind frame into a fixed-size stack buffer using the idiom `cw = snprintf(temp, remaining, ...); temp += cw;` without ever clamping `cw` or checking for truncation. Because `snprintf()` returns the number of bytes that *would* have been written, a sufficiently deep call stack at the moment of a SIGSEGV/SIGABRT pushes `temp` past the end of `msg[]`, turns the remaining-size argument negative (which is then implicitly converted to a huge `size_t`), and causes the next `snprintf()` call to write symbol-name bytes onto out-of-bounds stack memory. The defect can only fire while the process is already crashing, so the practical impact is that an already-fatal signal is mis-handled — the diagnostic backtrace is corrupted or the process dies from a secondary fault — rather than a standalone, network-reachable overflow.

## Affected Piece of Code

- **File:** `src/suricata.c`
- **Function / Location:** `SignalHandlerUnexpected()` ~L336-377
- **Subsystem:** unix-runmodes — Unix socket command server, runmodes, main, privilege drop, landlock

```c
349:    char *temp = msg;
350:    int cw = snprintf(temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), "stacktrace:sig %d:", sig_num);
351:    temp += cw;
352:    r = 1;
353:    while (r > 0) {
354:        if (unw_is_signal_frame(&cursor) == 0) {
355:            unw_word_t off;
356:            char name[256];
357:            if (unw_get_proc_name(&cursor, name, sizeof(name), &off) == UNW_ENOMEM) {
358:                cw = snprintf(temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), "[unknown]:");
359:            } else {
360:                cw = snprintf(
361:                        temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), "%s+0x%08" PRIx64, name, off);
362:            }
363:            temp += cw;
364:        }
365:
366:        r = unw_step(&cursor);
367:        if (r > 0) {
368:            cw = snprintf(temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), ";");
369:            temp += cw;
370:        }
371:    }
```

## The Bug

### Root cause

`SignalHandlerUnexpected()` is the `sa_sigaction` handler that Suricata installs for `SIGSEGV` and `SIGABRT` on non-Windows builds compiled with libunwind support. When invoked it walks the faulting thread's stack with `unw_step()` and serialises every non-signal frame into the local array `char msg[SC_LOG_MAX_LOG_MSG_LEN]` (L338). `SC_LOG_MAX_LOG_MSG_LEN` expands to `8192 + PATH_MAX + 512`, i.e. roughly 12 800 bytes on Linux where `PATH_MAX` is 4096.

Each iteration appends to the buffer with the pattern:

```c
cw = snprintf(temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), fmt, ...);
temp += cw;
```

This is the classic `snprintf` return-value misuse. Per ISO C, `snprintf` returns the number of characters that **would** have been written had the buffer been unlimited (excluding the terminating NUL), not the number actually stored. The code never checks `cw` against the remaining space, never tests for `cw < 0`, and never bounds the `temp += cw` advance. Once the cumulative formatted output reaches ~12 800 bytes:

1. `snprintf` truncates the output but still returns the full would-be length.
2. `temp += cw` moves `temp` strictly past `msg + SC_LOG_MAX_LOG_MSG_LEN`.
3. On the next call the size expression `SC_LOG_MAX_LOG_MSG_LEN - (temp - msg)` is computed as `int − ptrdiff_t → ptrdiff_t`, yielding a **negative** value.
4. That negative `ptrdiff_t` is passed as the second argument to `snprintf`, whose prototype takes `size_t`; the implicit conversion produces an enormous unsigned size (≈ 2⁶⁴ − k).
5. `snprintf` therefore believes it has effectively unlimited room and writes the entire next formatted frame string — `name`, `"+0x"`, 8-16 hex digits, and the trailing `";"` — at the out-of-bounds stack address `temp`, which now points beyond `msg[]` and into adjacent locals / saved registers / the saved return address of the signal-handler frame.

All subsequent loop iterations continue writing further and further past the buffer until `unw_step()` returns ≤ 0, after which `SCLogError("%s", msg)` is called and `kill(getpid(), sig_num)` (L376) re-raises the original signal.

A secondary defect at L357 widens the window: the code compares the return of `unw_get_proc_name()` against `UNW_ENOMEM`, but libunwind actually returns the **negated** error code `-UNW_ENOMEM` on truncation. The `if` branch therefore never fires, so even a symbol name that completely fills the 256-byte `name[]` buffer is printed via the `else` branch — guaranteeing that worst-case per-frame output is `255 (name) + 3 ("+0x") + 16 (hex) + 1 (";")` ≈ 275 bytes, and that an uninitialised `name[]` could be printed if `unw_get_proc_name()` failed for some other reason.

### Call chain — handler installation

The handler is registered unconditionally on the default configuration:

- `main()` — `src/main.c:20`
- → `SuricataInit()` — `src/suricata.c:3101`
- → `PostConfLoadedSetup()` — `src/suricata.c:2805` / called at `src/suricata.c:3148`
- → `InitSignalHandler()` — `src/suricata.c:2307`
- → `sigaction(SIGSEGV, &stacktrace_action, NULL)` and `sigaction(SIGABRT, &stacktrace_action, NULL)` with `.sa_sigaction = SignalHandlerUnexpected` — `src/suricata.c:2319-2326`

`InitSignalHandler()` consults `logging.stacktrace-on-signal` via `ConfGetBool()`; when the YAML key is absent the local `enabled` variable stays at its initialiser `1` (L2315-2317), so the handler is active on every non-Windows build that found libunwind at `./configure` time. The startup log line *"Preparing unexpected signal handling"* confirms registration.

### Call chain — runtime trigger

- Any Suricata thread dereferences a bad pointer, hits `BUG_ON()`/`abort()`, or otherwise faults.
- → kernel delivers `SIGSEGV` or `SIGABRT` to that thread.
- → `SignalHandlerUnexpected(sig_num, info, context)` — `src/suricata.c:336`
- → `unw_init_local(&cursor, (unw_context_t *)context)` — L344
- → loop: `unw_is_signal_frame()` / `unw_get_proc_name()` / `snprintf()` / `unw_step()` — L353-371
- → each iteration executes `temp += cw` with no clamp — L351, L363, L369

### Required field values / preconditions

The overflow is reached when the total formatted backtrace exceeds ~12 800 bytes. With worst-case 255-character symbol names that is **~47 frames**; with typical Suricata symbol lengths of 30-50 characters it is **~250-400 frames**. In other words, the bug requires a *separate, pre-existing crash primitive* that fires while the faulting thread is inside a deep or recursive call path — it is **not** directly reachable from a single crafted packet on its own. Plausible deep stacks inside Suricata include nested tunnel decoding (`decoder.max-layers`, default 16) composed with app-layer parsing, detection-engine evaluation, Lua rule scripts that recurse, or deeply nested PCRE/Hyperscan callback chains.

**Vulnerability class:** OOB-write (CWE-787, stack-based, via misuse of `snprintf` return value — CWE-131 contributing).

## Reproduction Results

**Status:** analytical only — a direct network trigger was not constructed.

**Blocker.** `SignalHandlerUnexpected()` executes only when the process is *already* receiving `SIGSEGV` or `SIGABRT`. Reaching the out-of-bounds write therefore requires chaining a *separate* memory-corruption or abort bug whose crash site sits ≥ ~47-400 frames deep. Identifying such a second bug is out of scope for this report; without it, no packet or PDU alone enters this code path.

**Analytical reproduction (demonstrates the arithmetic defect without a second bug).**

1. Build Suricata on Linux with `--enable-libunwind` (this is the default whenever `libunwind-dev` is present) and do **not** set `logging.stacktrace-on-signal: no` in `suricata.yaml`. Confirm the startup log line *"Preparing unexpected signal handling"*.
2. Driving the real ~12 800-byte buffer to overflow under a debugger is awkward — attaching gdb to a worker thread, breaking in a hot leaf such as `FlowGetFlowFromHash`, and scripting `signal SIGSEGV` after pushing ~500 synthetic frames is impractical. A far simpler proof of the arithmetic defect is to shrink the buffer: temporarily redefine `SC_LOG_MAX_LOG_MSG_LEN` to a small value (e.g. `#define SC_LOG_MAX_LOG_MSG_LEN 64`), rebuild with `-fsanitize=address`, start Suricata, and run `kill -SEGV $(pidof suricata)`. With only 3-4 frames the 64-byte buffer truncates, `temp` walks past `msg + 64`, the next size argument goes negative → huge `size_t`, and AddressSanitizer reports a **stack-buffer-overflow WRITE** in `SignalHandlerUnexpected` at `suricata.c:360`/`368` before the re-raised `SIGSEGV` terminates the process.
3. To observe the same behaviour with the real 12 800-byte buffer, deliver `SIGABRT` to a thread that is currently inside a deeply recursive code path — for example a Lua rule script that recurses into itself, or a pathological deeply-nested PCRE match — so that libunwind walks ≥ ~250 frames. ASan or Valgrind will flag the out-of-bounds store at L360-363 or L368.

**What a real attacker would need (chained-exploit recipe).**

- (a) A primary vulnerability that causes `SIGSEGV`/`SIGABRT` inside packet processing — any future parser NULL-dereference, heap overflow, or `BUG_ON()` reachable from traffic.
- (b) That fault must occur while the call stack is deep — most plausibly via recursive tunnel decoding (tunnel-in-tunnel up to `decoder.max-layers`, default 16) stacked on top of app-layer + detection-engine + Lua, or via a recursive regex / Hyperscan callback chain.
- (c) Suricata built with libunwind and `logging.stacktrace-on-signal` left at its default (enabled).

Given (a)-(c) the handler overruns `msg[]` on the signal-handler stack. The bytes written are libunwind-derived symbol strings and hex offsets — **not** attacker-chosen content — and `SIG_DFL` has already been restored at L341-342, so a fault during the overflow terminates immediately. The practical outcome is therefore a corrupted crash (wrong terminating signal, clobbered or missing crash log) rather than controlled code execution.

## Severity

**LOW** — This is a stack buffer overflow inside the `SIGSEGV`/`SIGABRT` handler. In practice it only changes *how* an already-fatal crash terminates: the process may die from the secondary overflow instead of cleanly logging the original backtrace and re-raising the signal, degrading post-mortem diagnostics.

The rating is constrained by four factors:

1. Execution only reaches this code when the process is **already crashing** — there is no independent entry point.
2. `SIG_DFL` is restored for both signals at L341-342, so any fault triggered by the overflow itself terminates the process immediately rather than re-entering the handler.
3. `kill(getpid(), sig_num)` at L376 would terminate the process anyway even if the overflow were silently absorbed.
4. The overflowed bytes are libunwind-derived symbol names and offsets, **not** attacker-controlled data, so the content of the out-of-bounds write cannot be steered.

In the theoretical worst case the defect could convert a benign `SIGABRT` (e.g. from a `BUG_ON()` assertion) into stack corruption that overwrites the saved return address of the handler frame with symbol-string bytes, but the process is on a one-way path to termination regardless. This is therefore best characterised as a robustness / crash-escalation defect rather than a standalone RCE or DoS vector.

## Suggested Fix

Clamp every `snprintf` result and stop iterating once the buffer is full, mirroring the guard already used in `src/util-debug.c:385`. Concretely:

```c
    char *temp = msg;
    char *end  = msg + sizeof(msg);
    int cw = snprintf(temp, end - temp, "stacktrace:sig %d:", sig_num);
    if (cw < 0) goto log;
    temp += MIN(cw, (int)(end - temp - 1));
    r = 1;
    while (r > 0 && temp < end - 1) {
        if (unw_is_signal_frame(&cursor) == 0) {
            unw_word_t off;
            char name[256] = "?";
            if (unw_get_proc_name(&cursor, name, sizeof(name), &off) == -UNW_ENOMEM) {
                cw = snprintf(temp, end - temp, "[unknown]");
            } else {
                cw = snprintf(temp, end - temp, "%s+0x%08" PRIx64, name, off);
            }
            if (cw < 0) break;
            temp += MIN(cw, (int)(end - temp - 1));
        }
        r = unw_step(&cursor);
        if (r > 0 && temp < end - 1) {
            *temp++ = ';';
            *temp = '\0';
        }
    }
log:
    SCLogError("%s", msg);
```

Key changes:

- **(a)** Compute remaining space as `end - temp` from a fixed `end` pointer so the size argument can never go negative.
- **(b)** Check `cw < 0` and bail out on encoding errors.
- **(c)** Advance `temp` by `MIN(cw, remaining - 1)` so the cursor never passes the NUL terminator.
- **(d)** Add `temp < end - 1` to the `while` condition so iteration stops as soon as the buffer is full instead of continuing to call `snprintf` with zero/negative space.
- **(e)** Fix the `UNW_ENOMEM` comparison to `-UNW_ENOMEM` (libunwind returns negated error codes) and zero-initialise `name[]` so a non-`ENOMEM` failure does not print uninitialised stack bytes.