Fixed out-of-bounds read in module protocol when a '%' line has no '='

libpromises/evalfunction.c

@@ -10246,7 +10246,10 @@ void ModuleProtocol(EvalContext *ctx, const char *command, const char *line, int
         // context name once it's in the vartable.  Maybe this can be relaxed.
         sscanf(line + 1, "%256[^=]=", name);
 
-        if (CheckID(name))
+        /* A well-formed line is "%name=<json>". Without the '=' delimiter
+         * sscanf() leaves the whole tail in name, so strlen(name) == length-1
+         * and the "length - strlen(name) - 1 - 1" below underflows. */
+        if (CheckID(name) && line[strlen(name) + 1] == '=')
         {
             JsonElement *json = NULL;
             Buffer *holder = BufferNewFrom(line+strlen(name)+1+1,

tests/unit/evalfunction_test.c

@@ -7,6 +7,13 @@
 #include <eval_context.h>
 #include <evalfunction.c>
 
+#ifndef __MINGW32__
+#include <sys/mman.h>
+#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON)
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+#endif
+
 static bool netgroup_more = false;
 
 #if SETNETGRENT_RETURNS_INT
@@ -126,13 +133,61 @@ static void test_basename(void)
     basename_single_testcase("//a//b///c.csv////", ".csv", "c");
 }
 
+static void test_module_protocol_percent_no_delimiter(void)
+{
+    EvalContext *ctx = EvalContextNew();
+    StringSet *tags = StringSetNew();
+    long persistence = 0;
+    char context[CF_BUFSIZE] = "test";
+
+    /* A well-formed '%name=<json>' line still defines the container, so the
+     * added delimiter check does not reject valid input. */
+    char *ok = xstrdup("%good={\"k\":1}");
+    ModuleProtocol(ctx, "/dev/null", ok, false, context, sizeof(context),
+                   tags, &persistence);
+    VarRef *good = VarRefParseFromScope("good", context);
+    assert_true(EvalContextVariableGet(ctx, good, NULL) != NULL);
+    VarRefDestroy(good);
+    free(ok);
+
+#ifndef __MINGW32__
+    /* A '%' line with no '=' makes "length - strlen(name) - 1 - 1" underflow
+     * to SIZE_MAX and steps the source pointer one past the terminating NUL,
+     * so BufferAppend() scans off the end of the line buffer. Place the line
+     * so its NUL is the last readable byte before a PROT_NONE guard page: the
+     * unpatched code walks into the guard page (crashes), the patched code
+     * skips the line. */
+    const size_t pagesize = (size_t) sysconf(_SC_PAGESIZE);
+    char *region = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE,
+                        MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+    assert_true(region != MAP_FAILED);
+    assert_int_equal(mprotect(region + pagesize, pagesize, PROT_NONE), 0);
+
+    const char *bad = "%bad";
+    char *line = region + pagesize - strlen(bad) - 1; /* NUL at page boundary */
+    memcpy(line, bad, strlen(bad) + 1);
+
+    ModuleProtocol(ctx, "/dev/null", line, false, context, sizeof(context),
+                   tags, &persistence);
+    VarRef *ref = VarRefParseFromScope("bad", context);
+    assert_true(EvalContextVariableGet(ctx, ref, NULL) == NULL);
+    VarRefDestroy(ref);
+
+    assert_int_equal(munmap(region, pagesize * 2), 0);
+#endif
+
+    StringSetDestroy(tags);
+    EvalContextDestroy(ctx);
+}
+
 int main()
 {
     PRINT_TEST_BANNER();
     const UnitTest tests[] = {
         unit_test(test_hostinnetgroup_found),
         unit_test(test_hostinnetgroup_not_found),
         unit_test(test_basename),
+        unit_test(test_module_protocol_percent_no_delimiter),
     };
 
     return run_tests(tests);