Limit the number of transformers per rule or input (#309)

DataDog · Jun 24, 2024 · 15a3588 · 15a3588
1 parent aaf4d2a
commit 15a3588
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 5 deletions.
diff --git a/src/parser/expression_parser.cpp b/src/parser/expression_parser.cpp
@@ -19,7 +19,8 @@ namespace {
 
 template <typename T>
 std::vector<condition_parameter> parse_arguments(const parameter::map &params, data_source source,
-    const std::vector<transformer_id> &transformers, address_container &addresses)
+    const std::vector<transformer_id> &transformers, address_container &addresses,
+    const object_limits &limits)
 {
     const auto &specification = T::arguments();
     std::vector<condition_parameter> definitions;
@@ -67,6 +68,10 @@ std::vector<condition_parameter> parse_arguments(const parameter::map &params, d
                     address, get_target_index(address), std::move(kp), transformers, source});
             } else {
                 auto input_transformers = static_cast<parameter::vector>(it->second);
+                if (input_transformers.size() > limits.max_transformers_per_address) {
+                    throw ddwaf::parsing_error("number of transformers beyond allowed limit");
+                }
+
                 source = data_source::values;
                 auto new_transformers = parse_transformers(input_transformers, source);
                 targets.emplace_back(condition_target{address, get_target_index(address),
@@ -93,15 +98,16 @@ std::shared_ptr<expression> parse_expression(const parameter::vector &conditions
         auto params = at<parameter::map>(root, "parameters");
 
         if (operator_name == "lfi_detector") {
-            auto arguments = parse_arguments<lfi_detector>(params, source, transformers, addresses);
+            auto arguments =
+                parse_arguments<lfi_detector>(params, source, transformers, addresses, limits);
             conditions.emplace_back(std::make_unique<lfi_detector>(std::move(arguments), limits));
         } else if (operator_name == "ssrf_detector") {
             auto arguments =
-                parse_arguments<ssrf_detector>(params, source, transformers, addresses);
+                parse_arguments<ssrf_detector>(params, source, transformers, addresses, limits);
             conditions.emplace_back(std::make_unique<ssrf_detector>(std::move(arguments), limits));
         } else if (operator_name == "sqli_detector") {
             auto arguments =
-                parse_arguments<sqli_detector>(params, source, transformers, addresses);
+                parse_arguments<sqli_detector>(params, source, transformers, addresses, limits);
             conditions.emplace_back(std::make_unique<sqli_detector>(std::move(arguments), limits));
         } else {
             auto [data_id, matcher] = parse_matcher(operator_name, params);
@@ -111,7 +117,7 @@ std::shared_ptr<expression> parse_expression(const parameter::vector &conditions
             }
 
             auto arguments =
-                parse_arguments<scalar_condition>(params, source, transformers, addresses);
+                parse_arguments<scalar_condition>(params, source, transformers, addresses, limits);
 
             conditions.emplace_back(std::make_unique<scalar_condition>(
                 std::move(matcher), data_id, std::move(arguments), limits));

diff --git a/src/parser/parser_v1.cpp b/src/parser/parser_v1.cpp
@@ -123,6 +123,10 @@ void parseRule(parameter::map &rule, base_section_info &info,
     try {
         std::vector<transformer_id> rule_transformers;
         auto transformers = at<parameter::vector>(rule, "transformers", parameter::vector());
+        if (transformers.size() > limits.max_transformers_per_address) {
+            throw ddwaf::parsing_error("number of transformers beyond allowed limit");
+        }
+
         for (const auto &transformer_param : transformers) {
             auto transformer = static_cast<std::string_view>(transformer_param);
             auto id = transformer_from_string(transformer);

diff --git a/src/parser/rule_parser.cpp b/src/parser/rule_parser.cpp
@@ -19,6 +19,10 @@ rule_spec parse_rule(parameter::map &rule,
     std::vector<transformer_id> rule_transformers;
     auto data_source = ddwaf::data_source::values;
     auto transformers = at<parameter::vector>(rule, "transformers", {});
+    if (transformers.size() > limits.max_transformers_per_address) {
+        throw ddwaf::parsing_error("number of transformers beyond allowed limit");
+    }
+
     rule_transformers = parse_transformers(transformers, data_source);
 
     auto conditions_array = at<parameter::vector>(rule, "conditions");

diff --git a/src/utils.hpp b/src/utils.hpp
@@ -52,6 +52,7 @@ struct object_limits {
     uint32_t max_container_depth{DDWAF_MAX_CONTAINER_DEPTH};
     uint32_t max_container_size{DDWAF_MAX_CONTAINER_SIZE};
     uint32_t max_string_length{DDWAF_MAX_STRING_LENGTH};
+    uint32_t max_transformers_per_address{10}; // can't be overridden for now
 };
 
 using target_index = std::size_t;

diff --git a/tests/parser_v1_test.cpp b/tests/parser_v1_test.cpp
@@ -324,4 +324,43 @@ TEST(TestParserV1, TestInvalidDuplicate)
     ddwaf_destroy(handle);
 }
 
+TEST(TestParserV1, TestInvalidTooManyTransformers)
+{
+    auto rule = read_file("invalid_too_many_transformers_v1.yaml");
+    ASSERT_TRUE(rule.type != DDWAF_OBJ_INVALID);
+
+    ddwaf_object diagnostics;
+
+    ddwaf_handle handle = ddwaf_init(&rule, nullptr, &diagnostics);
+    ASSERT_EQ(handle, nullptr);
+    ddwaf_object_free(&rule);
+
+    ddwaf::parameter root(diagnostics);
+    auto root_map = static_cast<ddwaf::parameter::map>(root);
+
+    auto version = ddwaf::parser::at<std::string>(root_map, "ruleset_version", "");
+    EXPECT_STREQ(version.c_str(), "");
+
+    auto rules = ddwaf::parser::at<parameter::map>(root_map, "rules");
+
+    auto loaded = ddwaf::parser::at<parameter::vector>(rules, "loaded");
+    EXPECT_EQ(loaded.size(), 0);
+
+    auto failed = ddwaf::parser::at<parameter::vector>(rules, "failed");
+    EXPECT_EQ(failed.size(), 1);
+
+    auto errors = ddwaf::parser::at<parameter::map>(rules, "errors");
+    EXPECT_EQ(errors.size(), 1);
+
+    auto it = errors.find("number of transformers beyond allowed limit");
+    EXPECT_NE(it, errors.end());
+
+    auto error_rules = static_cast<ddwaf::parameter::string_set>(it->second);
+    EXPECT_EQ(error_rules.size(), 1);
+    EXPECT_NE(error_rules.find("1"), error_rules.end());
+
+    ddwaf_object_free(&diagnostics);
+    ddwaf_destroy(handle);
+}
+
 } // namespace
diff --git a/tests/parser_v2_rules_test.cpp b/tests/parser_v2_rules_test.cpp
@@ -368,4 +368,87 @@ TEST(TestParserV2Rules, ParseMultipleRulesOneDuplicate)
         EXPECT_STR(rule.tags["category"], "category1");
     }
 }
+
+TEST(TestParserV2Rules, ParseSingleRuleTooManyTransformers)
+{
+    ddwaf::object_limits limits;
+    ddwaf::ruleset_info::section_info section;
+    std::unordered_map<std::string, std::string> rule_data_ids;
+
+    auto rule_object = yaml_to_object(
+        R"([{id: 1, name: rule1, tags: {type: flow1, category: category1}, conditions: [{operator: match_regex, parameters: {inputs: [{address: arg1}], regex: .*}}, {operator: match_regex, parameters: {inputs: [{address: arg2, key_path: [x]}], regex: .*}}, {operator: match_regex, parameters: {inputs: [{address: arg2, key_path: [y]}], regex: .*}}], transformers: [base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode]}])");
+
+    auto rule_array = static_cast<parameter::vector>(parameter(rule_object));
+    EXPECT_EQ(rule_array.size(), 1);
+
+    auto rules = parser::v2::parse_rules(rule_array, section, rule_data_ids, limits);
+    ddwaf_object_free(&rule_object);
+
+    {
+        ddwaf::parameter root;
+        section.to_object(root);
+
+        auto root_map = static_cast<parameter::map>(root);
+
+        auto loaded = ddwaf::parser::at<parameter::string_set>(root_map, "loaded");
+        EXPECT_EQ(loaded.size(), 0);
+
+        auto failed = ddwaf::parser::at<parameter::string_set>(root_map, "failed");
+        EXPECT_EQ(failed.size(), 1);
+        EXPECT_NE(failed.find("1"), failed.end());
+
+        auto errors = ddwaf::parser::at<parameter::map>(root_map, "errors");
+        EXPECT_EQ(errors.size(), 1);
+        auto it = errors.find("number of transformers beyond allowed limit");
+        EXPECT_NE(it, errors.end());
+
+        auto error_rules = static_cast<ddwaf::parameter::string_set>(it->second);
+        EXPECT_EQ(error_rules.size(), 1);
+        EXPECT_NE(error_rules.find("1"), error_rules.end());
+
+        ddwaf_object_free(&root);
+    }
+}
+
+TEST(TestParserV2Rules, ParseSingleRuleTooManyInputTransformers)
+{
+    ddwaf::object_limits limits;
+    ddwaf::ruleset_info::section_info section;
+    std::unordered_map<std::string, std::string> rule_data_ids;
+
+    auto rule_object = yaml_to_object(
+        R"([{id: 1, name: rule1, tags: {type: flow1, category: category1}, conditions: [{operator: match_regex, parameters: {inputs: [{address: arg1}], regex: .*}}, {operator: match_regex, parameters: {inputs: [{address: arg2, key_path: [x]}], regex: .*}}, {operator: match_regex, parameters: {inputs: [{address: arg2, key_path: [y], transformers: [base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode, base64_encode]}], regex: .*}}]}])");
+
+    auto rule_array = static_cast<parameter::vector>(parameter(rule_object));
+    EXPECT_EQ(rule_array.size(), 1);
+
+    auto rules = parser::v2::parse_rules(rule_array, section, rule_data_ids, limits);
+    ddwaf_object_free(&rule_object);
+
+    {
+        ddwaf::parameter root;
+        section.to_object(root);
+
+        auto root_map = static_cast<parameter::map>(root);
+
+        auto loaded = ddwaf::parser::at<parameter::string_set>(root_map, "loaded");
+        EXPECT_EQ(loaded.size(), 0);
+
+        auto failed = ddwaf::parser::at<parameter::string_set>(root_map, "failed");
+        EXPECT_EQ(failed.size(), 1);
+        EXPECT_NE(failed.find("1"), failed.end());
+
+        auto errors = ddwaf::parser::at<parameter::map>(root_map, "errors");
+        EXPECT_EQ(errors.size(), 1);
+        auto it = errors.find("number of transformers beyond allowed limit");
+        EXPECT_NE(it, errors.end());
+
+        auto error_rules = static_cast<ddwaf::parameter::string_set>(it->second);
+        EXPECT_EQ(error_rules.size(), 1);
+        EXPECT_NE(error_rules.find("1"), error_rules.end());
+
+        ddwaf_object_free(&root);
+    }
+}
+
 } // namespace
diff --git a/tests/yaml/invalid_too_many_transformers_v1.yaml b/tests/yaml/invalid_too_many_transformers_v1.yaml
@@ -0,0 +1,29 @@
+version: '1.1'
+events:
+  - id: 1
+    name: rule1
+    tags:
+      category: category1
+    conditions:
+      - operation: match_regex
+        parameters:
+          inputs:
+            - arg1
+          regex: .*
+      - operation: match_regex
+        parameters:
+          inputs:
+            - arg2
+          regex: .*
+    transformers:
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode
+      - base64_encode