From: Alan T. DeKok <aland@freeradius.org>
Date: Tue, 19 May 2026 22:06:39 +0000 (-0500)
Subject: catch corner case in sbuff, and add test for it
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0ea91d4e580b60d06592c832a47330f9ab3db0a7;p=thirdparty%2Ffreeradius-server.git

catch corner case in sbuff, and add test for it
---

diff --git a/src/lib/util/sbuff.c b/src/lib/util/sbuff.c
index 7f1132b4c65..e078b759580 100644
--- a/src/lib/util/sbuff.c
+++ b/src/lib/util/sbuff.c
@@ -553,7 +553,7 @@ static inline CC_HINT(always_inline) void fr_sbuff_terminal_idx_init(size_t *nee
  */
 static inline bool fr_sbuff_terminal_search(fr_sbuff_t *in, char const *p,
 					    uint8_t idx[static SBUFF_CHAR_CLASS],
-					    fr_sbuff_term_t const *term, size_t needle_len)
+					    fr_sbuff_term_t const *term, UNUSED size_t needle_len)
 {
 	uint8_t 	term_idx;
 
@@ -570,23 +570,20 @@ static inline bool fr_sbuff_terminal_search(fr_sbuff_t *in, char const *p,
 	term_idx = idx[(uint8_t)*p];			/* Fast path */
 	if (!term_idx) return false;
 
+	if (p > in->end) return false; /* paranoia */
+
 	/*
-	 *	Special case for EOFlike states
+	 *	"p" may be ahead of "in->p", as the caller can scan forward without advancing "in->p`".  So we
+	 *	need to measure bytes available from "p".  Othwrwise using fr_sbuff_remaining(in) would
+	 *	over-state the available bytes by (p - in->p) and read past in->end.
 	 */
-	remaining = fr_sbuff_remaining(in);
-	if ((remaining == 0) && !fr_sbuff_is_extendable(in)) {
-		if (idx['\0'] != 0) return true;
-		return false;
-	}
+	remaining = (size_t)(in->end - p);
 
-	if (remaining < needle_len) {
-		fr_assert_msg(!fr_sbuff_is_extendable(in),
-			      "Caller failed to extend buffer by %zu bytes before calling fr_sbuff_terminal_search",
-			      needle_len);
-		/*
-		 *	We can't search for the needle if we don't have
-		 *	enough data to match it.
-		 */
+	/*
+	 *	Special case for EOFlike states
+	 */
+	if (remaining == 0) {
+		if (!fr_sbuff_is_extendable(in) && (idx['\0'] != 0)) return true;
 		return false;
 	}
 
diff --git a/src/lib/util/test/sbuff_tests.c b/src/lib/util/test/sbuff_tests.c
index f1ed1e48490..00c43949e65 100644
--- a/src/lib/util/test/sbuff_tests.c
+++ b/src/lib/util/test/sbuff_tests.c
@@ -759,6 +759,38 @@ static void test_eof_terminal(void)
 	TEST_CHECK(fr_sbuff_is_terminal(&sbuff, &tt) == false);
 }
 
+/*
+ *	fr_sbuff_terminal_search() is called with a `p` that walks
+ *	forward independently of in->p (see fr_sbuff_out_bstrncpy_until).
+ *	`remaining` must be measured from `p`, not from `in->p` — otherwise
+ *	memcmp() can read past in->end when matching a multi-byte terminal
+ *	near the visible end of the buffer.
+ *
+ *	The underlying C string is "abc}xyz" but the sbuff is initialised
+ *	to only see the first 4 bytes ("abc}").  With a "}xyz" terminal,
+ *	the (buggy) code would happily compare 4 bytes from p='}' against
+ *	"}xyz" — reading the "xyz" tail past in->end and reporting a false
+ *	match.  The fix clamps the compare to (in->end - p) = 1 byte, so
+ *	the partial-prefix match correctly falls through.
+ */
+static void test_terminal_search_past_visible_end(void)
+{
+	char const		in[] = "abc}xyz";
+	fr_sbuff_t		sbuff;
+	fr_sbuff_term_t		tt = FR_SBUFF_TERMS(
+					L("}xyz")
+				);
+	char			out[16];
+	ssize_t			slen;
+
+	fr_sbuff_init_in(&sbuff, in, (size_t)4);	/* Only "abc}" is visible */
+
+	slen = fr_sbuff_out_bstrncpy_until(&FR_SBUFF_OUT(out, sizeof(out)), &sbuff, SIZE_MAX, &tt, NULL);
+
+	TEST_CHECK_SLEN(slen, 4);
+	TEST_CHECK_STRCMP(out, "abc}");
+}
+
 static void test_terminal_merge(void)
 {
 	size_t i;
@@ -1604,6 +1636,7 @@ TEST_LIST = {
 	{ "multi-char terminals",		test_unescape_multi_char_terminals },
 	{ "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 merge",			test_terminal_merge },
 
 	/*