From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 11 Aug 2026 01:22:13 +0000 (-0400)
Subject: check for no data in sbuff terminal search
X-Git-Url: http://git.ipfire.org/index.cgi?a=commitdiff_plain;ds=inline;p=thirdparty%2Ffreeradius-server.git

check for no data in sbuff terminal search

before checking if the character is a terminal one.
---

diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c
index 6e6cb8fe09..b7c4c7a67a 100644
--- a/src/lib/util/sbuff.c
+++ b/src/lib/util/sbuff.c
@@ -565,11 +565,6 @@ static inline bool fr_sbuff_terminal_search(fr_sbuff_t *in, char const *p,
 
 	if (!term) return false;			/* If there's no terminals, we don't need to search */
 
-	end = term->len - 1;
-
-	term_idx = idx[(uint8_t)*p];			/* Fast path */
-	if (!term_idx) return false;
-
 	if (p > in->end) return false; /* paranoia */
 
 	/*
@@ -580,13 +575,20 @@ static inline bool fr_sbuff_terminal_search(fr_sbuff_t *in, char const *p,
 	remaining = (size_t)(in->end - p);
 
 	/*
-	 *	Special case for EOFlike states
+	 *	Special case for EOFlike states.
+	 *
+	 *	This MUST be checked before dereferencing "*p" below.  When the buffer is fully consumed, we
+	 *	have "p == in->end".  A dereference of "*p" is one byte past the end of the buffer, and would result in an overflow.
 	 */
 	if (remaining == 0) {
 		if (!fr_sbuff_is_extendable(in) && (idx['\0'] != 0)) return true;
 		return false;
 	}
 
+	term_idx = idx[(uint8_t)*p];			/* Fast path */
+	if (!term_idx) return false;
+
+	end = term->len - 1;
 	mid = term_idx - 1;				/* Inform the mid point from the index */
 
 	while (start <= end) {
diff --git a/src/lib/util/test/sbuff_tests.c b/src/lib/util/test/sbuff_tests.c
index 26857cfc8c..60cdd1b4da 100644
--- a/src/lib/util/test/sbuff_tests.c
+++ b/src/lib/util/test/sbuff_tests.c
@@ -789,6 +789,44 @@ static void test_terminal_search_past_visible_end(void)
 	TEST_CHECK_STRCMP(out, "abc}");
 }
 
+/*
+ *	Regression: fr_sbuff_terminal_search() did the fast-path "idx[*p]"
+ *	dereference before checking whether the buffer was fully consumed.
+ *	When "p == in->end" (nothing left) "*p" is one byte past the usable
+ *	buffer.  For a non-NUL-terminated sbuff whose end sits on an
+ *	allocation boundary that is a 1-byte over-read, which ASAN reports as
+ *	a heap-buffer-overflow.
+ *
+ *	A heap buffer sized exactly to its contents (no trailing '\0') is used
+ *	so the byte past "end" lands in an ASAN redzone.
+ */
+static void test_terminal_search_at_end_no_overrun(void)
+{
+	size_t const		len = 4;
+	char			*buff = malloc(len);	/* exactly len bytes, no NUL */
+	fr_sbuff_t		sbuff;
+	fr_sbuff_term_t		tt = FR_SBUFF_TERMS(
+					L(","),
+					L("}"),
+				);
+
+	TEST_CHECK(buff != NULL);
+	if (!buff) return;
+	memcpy(buff, "abcd", len);
+
+	fr_sbuff_init_in(&sbuff, buff, len);
+	fr_sbuff_advance(&sbuff, len);		/* Consume everything: in->p == in->end */
+
+	/*
+	 *	Must not read *p (one byte past the buffer).  The sbuff is not
+	 *	extendable and '\0' is not a terminal, so this is simply "not a
+	 *	terminal".
+	 */
+	TEST_CHECK(fr_sbuff_is_terminal(&sbuff, &tt) == false);
+
+	free(buff);
+}
+
 static void test_terminal_merge(void)
 {
 	size_t i;
@@ -1635,6 +1673,7 @@ TEST_LIST = {
 	{ "fr_sbuff_out_unescape_until",	test_unescape_until },
 	{ "fr_sbuff_terminal_eof",		test_eof_terminal },
 	{ "terminal search past visible end",	test_terminal_search_past_visible_end },
+	{ "terminal search at end no overrun",	test_terminal_search_at_end_no_overrun },
 	{ "terminal merge",			test_terminal_merge },
 
 	/*