diff --git a/libpromises/evalfunction.c b/libpromises/evalfunction.c index b65b514af1..f9d160528d 100644 --- a/libpromises/evalfunction.c +++ b/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=". 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, diff --git a/tests/unit/evalfunction_test.c b/tests/unit/evalfunction_test.c index b0ba60194a..16db5c1acd 100644 --- a/tests/unit/evalfunction_test.c +++ b/tests/unit/evalfunction_test.c @@ -7,6 +7,13 @@ #include #include +#ifndef __MINGW32__ +#include +#if !defined(MAP_ANONYMOUS) && defined(MAP_ANON) +#define MAP_ANONYMOUS MAP_ANON +#endif +#endif + static bool netgroup_more = false; #if SETNETGRENT_RETURNS_INT @@ -126,6 +133,53 @@ 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=' 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(); @@ -133,6 +187,7 @@ int main() 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);