From 0a1557cb3fe1e2df74a725eca59f7e07fc55971b Mon Sep 17 00:00:00 2001 From: Dhanraj E Date: Fri, 6 Nov 2026 15:11:04 +0530 Subject: [PATCH] Fixed out-of-bounds read in string_replace with nested list elements StringReplaceWithLists(), the string_replace(..., "lists") policy function, passes each element of the match and substitute arrays directly to JsonPrimitiveGetAsString(): const char *m = JsonPrimitiveGetAsString(JsonAt(match_list, i)); const char *s = JsonPrimitiveGetAsString(JsonAt(substitute_list, i)); That function returns primitive.value, which in the JsonElement union aliases container.children (a Seq *). For an element that is itself an array or object it therefore returns the internal container pointer reinterpreted as a char *, and SearchAndReplace() then reads it as a NUL-terminated string. FnCallStringReplace() only checks that the two arguments are arrays of equal length, not that every element is a primitive, so a nested element reaches the unchecked call. These lists commonly come from data containers (readjson, parsejson, mergedata, module output), so JSON with a nested element triggers it. With NDEBUG the read runs off the Seq object into adjacent heap memory and those bytes, including internal pointers, end up in the returned string; otherwise the assertion in JsonPrimitiveGetAsString() aborts, which cf-promises hits during policy validation. Reject any non-primitive element before the conversion, reusing the function's existing warn-and-fail path. Changelog: Title Signed-off-by: Dhanraj E --- libpromises/evalfunction.c | 21 ++++++++++++++-- tests/unit/evalfunction_test.c | 44 ++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/libpromises/evalfunction.c b/libpromises/evalfunction.c index d42df8c6fca..87aea35969c 100644 --- a/libpromises/evalfunction.c +++ b/libpromises/evalfunction.c @@ -9164,8 +9164,25 @@ static char *StringReplaceWithLists(const char *input_string, JsonElement *match const size_t len = JsonLength(match_list); for (size_t i = 0; i < len; i++) { - const char *m = JsonPrimitiveGetAsString(JsonAt(match_list, i)); - const char *s = JsonPrimitiveGetAsString(JsonAt(substitute_list, i)); + JsonElement *match_el = JsonAt(match_list, i); + JsonElement *substitute_el = JsonAt(substitute_list, i); + + /* The elements must be primitives. JsonPrimitiveGetAsString() returns + * the aliased container pointer for a nested array or object, which is + * then used as a C string. */ + if (JsonGetElementType(match_el) != JSON_ELEMENT_TYPE_PRIMITIVE || + JsonGetElementType(substitute_el) != JSON_ELEMENT_TYPE_PRIMITIVE) + { + Log(LOG_LEVEL_WARNING, + "Failed to replace with function '%s', string: '%s': " + "match and substitute lists must contain only strings", + fp->name, input_string); + free(ret); + return NULL; + } + + const char *m = JsonPrimitiveGetAsString(match_el); + const char *s = JsonPrimitiveGetAsString(substitute_el); char *next = SearchAndReplace(ret, m, s); free(ret); diff --git a/tests/unit/evalfunction_test.c b/tests/unit/evalfunction_test.c index fcfd2f18a53..70d04f0dbf2 100644 --- a/tests/unit/evalfunction_test.c +++ b/tests/unit/evalfunction_test.c @@ -214,6 +214,49 @@ static void test_format_rejects_unpassed_arguments(void) } } +static FnCallResult string_replace_lists_testcase(const char *string, + const char *match, + const char *substitute) +{ + EvalContext *ctx = EvalContextNew(); + Rlist *args = NULL; + RlistAppendScalar(&args, string); + RlistAppendScalar(&args, match); + RlistAppendScalar(&args, substitute); + RlistAppendScalar(&args, "lists"); + FnCall *call = FnCallNew("string_replace", args); + + FnCallResult res = FnCallStringReplace(ctx, NULL, call, args); + + FnCallDestroy(call); + EvalContextDestroy(ctx); + return res; +} + +static void test_string_replace_rejects_non_primitive_list_elements(void) +{ + /* Valid flat lists still work. */ + FnCallResult res = + string_replace_lists_testcase("abcb", "[\"b\"]", "[\"Z\"]"); + assert_int_equal(res.status, FNCALL_SUCCESS); + assert_string_equal("aZcZ", (char *) res.rval.item); + RvalDestroy(res.rval); + + /* A substitute element that is a nested array passes the outer array and + * equal-length checks. The unpatched code passed it to + * JsonPrimitiveGetAsString(), which returned the aliased container pointer + * as a char *, and used it as a string (heap over-read / leak). */ + res = string_replace_lists_testcase("abcb", "[\"b\"]", "[[\"Z\"]]"); + assert_int_equal(res.status, FNCALL_FAILURE); + + /* Same on the match side, and for a nested object. */ + res = string_replace_lists_testcase("abcb", "[[\"b\"]]", "[\"Z\"]"); + assert_int_equal(res.status, FNCALL_FAILURE); + + res = string_replace_lists_testcase("abcb", "[\"b\"]", "[{\"k\":\"v\"}]"); + assert_int_equal(res.status, FNCALL_FAILURE); +} + int main() { PRINT_TEST_BANNER(); @@ -223,6 +266,7 @@ int main() unit_test(test_basename), unit_test(test_module_protocol_percent_no_delimiter), unit_test(test_format_rejects_unpassed_arguments), + unit_test(test_string_replace_rejects_non_primitive_list_elements), }; return run_tests(tests);