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 7f1132b4c65ebfef7cb887384f521db1d44b9492..e078b759580ee71740bda6b24fff90c8162fc3d3 100644 (file)
--- 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 f1ed1e48490764c7e384839a5c49630a69a318cc..00c43949e6553464bc7645c622574ad8747cdeeb 100644 (file)
--- 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 },
 
        /*