From cd7f7751d5174f8bb91767ca428251d16f6239a4 Mon Sep 17 00:00:00 2001 From: Jeff Lucovsky Date: Sun, 30 Aug 2026 09:17:27 -0400 Subject: [PATCH] suricata: bound stack trace formatting in signal handler The crash handler built its stack trace by advancing a pointer with the snprintf return value. That value is the length the output would have had, not what was written, so once the trace filled msg the pointer moved past the end of the buffer. Every size argument after that, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), was negative and converted to a huge size_t, and snprintf went on writing into the stack past msg. With a deep enough stack the handler faults before it can log the crash it exists to report. The frame name lookup also compared unw_get_proc_name() against UNW_ENOMEM, but libunwind returns the negated code, so the test never fired. A lookup that failed with UNW_ENOINFO fell through to the success branch and printed the name buffer, which libunwind had not written. Track space used with offset and use that on each snprintf call. Advance by MIN(cw, remaining - 1) to prevent overruns. The loop stops when the buffer is full and closes the trace with "..." so a clipped message is not read as a whole stack. Failed symbol lookups now print "[unknown]:". -UNW_ENOMEM is not one of them, since libunwind returns a truncated but usable name with it. name starts as "?" so nothing is printed from it either way. Ticket: 8846 --- src/suricata.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/suricata.c b/src/suricata.c index 14b3f4d6b1..2ab10b3d92 100644 --- a/src/suricata.c +++ b/src/suricata.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2025 Open Information Security Foundation +/* Copyright (C) 2007-2026 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -346,29 +346,38 @@ static void SignalHandlerUnexpected(int sig_num, siginfo_t *info, void *context) goto terminate; } - char *temp = msg; - int cw = snprintf(temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), "stacktrace:sig %d:", sig_num); - temp += cw; + int cw = snprintf(msg, sizeof(msg), "stacktrace:sig %d:", sig_num); + if (cw < 0) + goto terminate; + size_t offset = MIN((size_t)cw, sizeof(msg) - 1); + r = 1; - while (r > 0) { + while (r > 0 && offset < sizeof(msg) - 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, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), "[unknown]:"); + char name[256] = "?"; + int ret = unw_get_proc_name(&cursor, name, sizeof(name), &off); + /* -UNW_ENOMEM means the name was truncated to fit; it is still usable. */ + if (ret != 0 && ret != -UNW_ENOMEM) { + cw = snprintf(msg + offset, sizeof(msg) - offset, "[unknown]:"); } else { - cw = snprintf( - temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), "%s+0x%08" PRIx64, name, off); + cw = snprintf(msg + offset, sizeof(msg) - offset, "%s+0x%08" PRIx64, name, off); } - temp += cw; + if (cw < 0) + break; + offset += MIN((size_t)cw, sizeof(msg) - offset - 1); } r = unw_step(&cursor); - if (r > 0) { - cw = snprintf(temp, SC_LOG_MAX_LOG_MSG_LEN - (temp - msg), ";"); - temp += cw; + if (r > 0 && offset < sizeof(msg) - 1) { + msg[offset++] = ';'; + msg[offset] = '\0'; } } + /* Mark a trace that ran out of room so a clipped tail is not read as a + * complete stack. */ + if (offset >= sizeof(msg) - 1) + memcpy(msg + sizeof(msg) - 4, "...", 4); SCLogError("%s", msg); terminate: