From 31b6bfb059765fc232ff23c16d6057510d14b527 Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Mon, 22 Jan 2024 10:38:48 +0000 Subject: [PATCH 1/2] fix(userspace/engine): avoid storing escaped strings in engine defs Signed-off-by: Jason Dellaluce --- userspace/engine/rule_loader_compiler.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/userspace/engine/rule_loader_compiler.cpp b/userspace/engine/rule_loader_compiler.cpp index b16a8eea5e7..b45c14d861a 100644 --- a/userspace/engine/rule_loader_compiler.cpp +++ b/userspace/engine/rule_loader_compiler.cpp @@ -181,6 +181,7 @@ static bool resolve_list(std::string& cnd, const falco_list& list) { static std::string blanks = " \t\n\r"; static std::string delims = blanks + "(),="; + std::string tmp; std::string new_cnd; size_t start, end; bool used = false; @@ -212,7 +213,9 @@ static bool resolve_list(std::string& cnd, const falco_list& list) { sub += ", "; } - sub += v; + tmp = v; + quote_item(tmp); + sub += tmp; } // if substituted list is empty, we need to // remove a comma from the left or the right @@ -339,7 +342,6 @@ void rule_loader::compiler::compile_list_infos( const collector& col, indexed_vector& out) const { - std::string tmp; std::list used; falco_list v; for (const auto &list : col.lists()) @@ -352,17 +354,14 @@ void rule_loader::compiler::compile_list_infos( if (ref && ref->index < list.visibility) { used.push_back(ref->name); - for (auto val : ref->items) + for (const auto &val : ref->items) { - quote_item(val); v.items.push_back(val); } } else { - tmp = item; - quote_item(tmp); - v.items.push_back(tmp); + v.items.push_back(item); } } v.used = false; From 0615b2be61f3464b2d21fb114c01f43bd4f6b99c Mon Sep 17 00:00:00 2001 From: Jason Dellaluce Date: Mon, 22 Jan 2024 17:53:03 +0000 Subject: [PATCH 2/2] test(engine): assess proper list escaping in engine collector Signed-off-by: Jason Dellaluce --- unit_tests/engine/test_rule_loader.cpp | 27 ++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/unit_tests/engine/test_rule_loader.cpp b/unit_tests/engine/test_rule_loader.cpp index a6b7d1da2eb..28366020409 100644 --- a/unit_tests/engine/test_rule_loader.cpp +++ b/unit_tests/engine/test_rule_loader.cpp @@ -941,3 +941,30 @@ TEST_F(engine_loader_test, required_engine_version_invalid) ASSERT_FALSE(load_rules(rules_content, "rules.yaml")); ASSERT_TRUE(check_error_message("Unable to parse engine version")); } + +// checks for issue described in https://github.com/falcosecurity/falco/pull/3028 +TEST_F(engine_loader_test, list_value_with_escaping) +{ + std::string rules_content = R"END( +- list: my_list + items: [non_escaped_val, "escaped val"] +)END"; + + ASSERT_TRUE(load_rules(rules_content, "rules.yaml")); + ASSERT_TRUE(m_load_result->successful()); + ASSERT_TRUE(m_load_result->has_warnings()); // a warning for the unused list + + auto rule_description = m_engine->describe_rule(nullptr, {}); + ASSERT_TRUE(m_load_result->successful()); + ASSERT_EQ(rule_description["rules"].size(), 0); + ASSERT_EQ(rule_description["macros"].size(), 0); + ASSERT_EQ(rule_description["lists"].size(), 1); + + // escaped values must not be interpreted as list refs by mistake + ASSERT_EQ(rule_description["lists"][0]["details"]["lists"].size(), 0); + + // values should be escaped correctly + ASSERT_EQ(rule_description["lists"][0]["details"]["items_compiled"].size(), 2); + ASSERT_EQ(rule_description["lists"][0]["details"]["items_compiled"][0].template get(), "non_escaped_val"); + ASSERT_EQ(rule_description["lists"][0]["details"]["items_compiled"][1].template get(), "escaped val"); +} \ No newline at end of file