Phrase match: enforce word boundary option (#256)

Anilm3 · web-flow · commit 02da34d50d27 · 2024-02-07T14:04:41.000Z
diff --git a/benchmark/scenarios/phrase_match_matcher_enforce_word_boundary.json b/benchmark/scenarios/phrase_match_matcher_enforce_word_boundary.json
@@ -0,0 +1,49 @@
+{
+  "scenario": "phrase_match_matcher.enforce_word_boundary",
+  "ruleset": {
+    "rules": [
+      {
+        "id": "crs-913-110",
+        "name": "Acunetix",
+        "tags": {
+          "type": "commercial_scanner",
+          "category": "attack_attempt"
+        },
+        "conditions": [
+          {
+            "parameters": {
+              "inputs": [
+                {
+                  "address": "server.request.headers.no_cookies"
+                }
+              ],
+              "list": [
+                "acunetix-product",
+                "(acunetix web vulnerability scanner",
+                "acunetix-scanning-agreement",
+                "acunetix-user-agreement",
+                "md5(acunetix_wvs_security_test)"
+              ],
+              "options": {
+                  "enforce_word_boundary": true
+              }
+            },
+            "operator": "phrase_match"
+          }
+        ],
+        "transformers": []
+      }
+    ]
+  },
+  "fixtures": {
+    "eval.valid": {
+      "server.request.headers.no_cookies": [
+        "acunetix-product",
+        "(acunetix web vulnerability scanner",
+        "acunetix-scanning-agreement",
+        "acunetix-user-agreement",
+        "md5(acunetix_wvs_security_test)"
+      ]
+    }
+  }
+}
diff --git a/fuzzing/scripts/build_corpus.py b/fuzzing/scripts/build_corpus.py
@@ -391,6 +391,9 @@ def get_random_condition(i):
 
             if operator == "phrase_match":
                 result["parameters"]["list"] = [get_random_value(addresses) for _ in range(randint(1, 200))]
+                result["parameters"]["options"] = {
+                  "enforce_word_boundary": choice((True, False))
+                }
             elif operator == "match_regex":
                 temp = choice(self.regexs)
 
diff --git a/src/matcher/phrase_match.cpp b/src/matcher/phrase_match.cpp
@@ -12,7 +12,19 @@
 
 namespace ddwaf::matcher {
 
-phrase_match::phrase_match(std::vector<const char *> pattern, std::vector<uint32_t> lengths)
+namespace {
+bool is_bounded_word(std::string_view pattern, std::size_t begin, std::size_t end)
+{
+    return ((end + 1 >= pattern.size()) || isboundary(pattern[end]) ||
+               isboundary(pattern[end + 1])) &&
+           (begin == 0 || (isboundary(pattern[begin]) || isboundary(pattern[begin - 1])));
+}
+
+} // namespace
+
+phrase_match::phrase_match(
+    std::vector<const char *> pattern, std::vector<uint32_t> lengths, bool enforce_word_boundary)
+    : enforce_word_boundary_(enforce_word_boundary)
 {
     if (pattern.size() != lengths.size()) {
         throw std::invalid_argument("inconsistent pattern and lengths array size");
@@ -33,22 +45,27 @@ std::pair<bool, std::string> phrase_match::match_impl(std::string_view pattern)
         return {false, {}};
     }
 
-    const ac_result_t result =
-        ac_match(acStructure, pattern.data(), static_cast<uint32_t>(pattern.size()));
+    auto u32_size = static_cast<uint32_t>(pattern.size());
+    ac_result_t result;
+    if (!enforce_word_boundary_) {
+        result = ac_match(acStructure, pattern.data(), u32_size);
+    } else {
+        result = ac_match_longest_l(acStructure, pattern.data(), u32_size);
+    }
+
+    auto begin = static_cast<std::size_t>(result.match_begin);
+    auto end = static_cast<std::size_t>(result.match_end);
 
-    const bool didMatch =
-        result.match_begin >= 0 && result.match_end >= 0 && result.match_begin < result.match_end;
-    if (!didMatch) {
+    if (result.match_begin < 0 || result.match_end < 0 || begin >= end ||
+        (enforce_word_boundary_ && !is_bounded_word(pattern, begin, end))) {
         return {false, {}};
     }
 
-    std::string matched_value;
-    if (pattern.size() > static_cast<std::size_t>(result.match_end)) {
-        matched_value =
-            pattern.substr(result.match_begin, (result.match_end - result.match_begin + 1));
+    if (pattern.size() <= end) [[unlikely]] {
+        return {true, {}};
     }
 
-    return {true, matched_value};
+    return {true, std::string{pattern.substr(begin, (end - begin + 1))}};
 }
 
 } // namespace ddwaf::matcher
diff --git a/src/matcher/phrase_match.hpp b/src/matcher/phrase_match.hpp
@@ -15,7 +15,8 @@ namespace ddwaf::matcher {
 
 class phrase_match : public base_impl<phrase_match> {
 public:
-    phrase_match(std::vector<const char *> pattern, std::vector<uint32_t> lengths);
+    phrase_match(std::vector<const char *> pattern, std::vector<uint32_t> lengths,
+        bool enforce_word_boundary = false);
     ~phrase_match() override = default;
     phrase_match(const phrase_match &) = delete;
     phrase_match(phrase_match &&) noexcept = default;
@@ -29,6 +30,7 @@ class phrase_match : public base_impl<phrase_match> {
 
     [[nodiscard]] std::pair<bool, std::string> match_impl(std::string_view pattern) const;
 
+    bool enforce_word_boundary_{false};
     std::unique_ptr<ac_t, void (*)(void *)> ac{nullptr, nullptr};
 
     friend class base_impl<phrase_match>;
diff --git a/src/parser/parser_v2.cpp b/src/parser/parser_v2.cpp
@@ -50,6 +50,8 @@ std::pair<std::string, std::unique_ptr<matcher::base>> parse_matcher(
 
     if (name == "phrase_match") {
         auto list = at<parameter::vector>(params, "list");
+        options = at<parameter::map>(params, "options", options);
+        auto word_boundary = at<bool>(options, "enforce_word_boundary", false);
 
         std::vector<const char *> patterns;
         std::vector<uint32_t> lengths;
@@ -66,7 +68,7 @@ std::pair<std::string, std::unique_ptr<matcher::base>> parse_matcher(
             lengths.push_back((uint32_t)pattern.nbEntries);
         }
 
-        matcher = std::make_unique<matcher::phrase_match>(patterns, lengths);
+        matcher = std::make_unique<matcher::phrase_match>(patterns, lengths, word_boundary);
     } else if (name == "match_regex") {
         auto regex = at<std::string>(params, "regex");
         options = at<parameter::map>(params, "options", options);
diff --git a/src/utils.hpp b/src/utils.hpp
@@ -112,6 +112,7 @@ inline bool isspace(char c)
 inline bool isupper(char c) { return static_cast<unsigned>(c) - 'A' < 26; }
 inline bool islower(char c) { return static_cast<unsigned>(c) - 'a' < 26; }
 inline bool isalnum(char c) { return isalpha(c) || isdigit(c); }
+inline bool isboundary(char c) { return !isalnum(c) && c != '_'; }
 inline char tolower(char c) { return isupper(c) ? static_cast<char>(c | 32) : c; }
 inline uint8_t from_hex(char c)
 {
diff --git a/tests/integration/matchers/test.cpp b/tests/integration/matchers/test.cpp
@@ -161,4 +161,89 @@ TEST(TestIntegrationOperation, FloatEquals)
     ddwaf_context_destroy(context);
     ddwaf_destroy(handle);
 }
+
+TEST(TestIntegrationOperation, PhraseMatch)
+{
+    auto rule = read_file("phrase_match.yaml", base_dir);
+    ASSERT_TRUE(rule.type != DDWAF_OBJ_INVALID);
+    ddwaf_handle handle = ddwaf_init(&rule, nullptr, nullptr);
+    ASSERT_NE(handle, nullptr);
+    ddwaf_object_free(&rule);
+
+    ddwaf_context context = ddwaf_context_init(handle);
+    ASSERT_NE(context, nullptr);
+
+    ddwaf_object map = DDWAF_OBJECT_MAP;
+    ddwaf_object value;
+    ddwaf_object_string(&value, "string00");
+    ddwaf_object_map_add(&map, "input1", &value);
+
+    ddwaf_result out;
+    ASSERT_EQ(ddwaf_run(context, &map, nullptr, &out, LONG_TIME), DDWAF_MATCH);
+    EXPECT_FALSE(out.timeout);
+    EXPECT_EVENTS(out, {.id = "1",
+                           .name = "rule1-phrase-match",
+                           .tags = {{"type", "flow"}, {"category", "category"}},
+                           .matches = {{.op = "phrase_match",
+                               .address = "input1",
+                               .value = "string00",
+                               .highlight = "string00"}}});
+
+    ddwaf_result_free(&out);
+    ddwaf_context_destroy(context);
+    ddwaf_destroy(handle);
+}
+
+TEST(TestIntegrationOperation, PhraseMatchWordBound)
+{
+    auto rule = read_file("phrase_match.yaml", base_dir);
+    ASSERT_TRUE(rule.type != DDWAF_OBJ_INVALID);
+    ddwaf_handle handle = ddwaf_init(&rule, nullptr, nullptr);
+    ASSERT_NE(handle, nullptr);
+    ddwaf_object_free(&rule);
+
+    {
+        ddwaf_context context = ddwaf_context_init(handle);
+        ASSERT_NE(context, nullptr);
+
+        ddwaf_object map = DDWAF_OBJECT_MAP;
+        ddwaf_object value;
+        ddwaf_object_string(&value, "string01;");
+        ddwaf_object_map_add(&map, "input2", &value);
+
+        ddwaf_result out;
+        ASSERT_EQ(ddwaf_run(context, &map, nullptr, &out, LONG_TIME), DDWAF_MATCH);
+        EXPECT_FALSE(out.timeout);
+        EXPECT_EVENTS(out, {.id = "2",
+                               .name = "rule2-phrase-match-word-bound",
+                               .tags = {{"type", "flow"}, {"category", "category"}},
+                               .matches = {{.op = "phrase_match",
+                                   .address = "input2",
+                                   .value = "string01;",
+                                   .highlight = "string01"}}});
+
+        ddwaf_result_free(&out);
+        ddwaf_context_destroy(context);
+    }
+
+    {
+        ddwaf_context context = ddwaf_context_init(handle);
+        ASSERT_NE(context, nullptr);
+
+        ddwaf_object map = DDWAF_OBJECT_MAP;
+        ddwaf_object value;
+        ddwaf_object_string(&value, "string010");
+        ddwaf_object_map_add(&map, "input2", &value);
+
+        ddwaf_result out;
+        ASSERT_EQ(ddwaf_run(context, &map, nullptr, &out, LONG_TIME), DDWAF_OK);
+        EXPECT_FALSE(out.timeout);
+
+        ddwaf_result_free(&out);
+        ddwaf_context_destroy(context);
+    }
+
+    ddwaf_destroy(handle);
+}
+
 } // namespace
diff --git a/tests/integration/matchers/yaml/phrase_match.yaml b/tests/integration/matchers/yaml/phrase_match.yaml
@@ -0,0 +1,30 @@
+version: '2.1'
+rules:
+  - id: 1
+    name: rule1-phrase-match
+    tags:
+      type: flow
+      category: category
+    conditions:
+      - operator: phrase_match
+        parameters:
+          inputs:
+            - address: input1
+          list:
+            - string00
+            - string01
+  - id: 2
+    name: rule2-phrase-match-word-bound
+    tags:
+      type: flow
+      category: category
+    conditions:
+      - operator: phrase_match
+        parameters:
+          inputs:
+            - address: input2
+          list:
+            - string00
+            - string01
+          options:
+            enforce_word_boundary: true
diff --git a/tests/matcher/phrase_match_test.cpp b/tests/matcher/phrase_match_test.cpp
@@ -95,6 +95,89 @@ TEST(TestPhraseMatch, TestComplex)
     run("nonsense", nullptr);
 }
 
+TEST(TestPhraseMatch, TestWordBoundary)
+{
+    std::vector<const char *> strings{"banana", "$apple", "orange$", "$pear$"};
+    std::vector<uint32_t> lengths(strings.size());
+    std::generate(lengths.begin(), lengths.end(),
+        [i = 0, &strings]() mutable { return strlen(strings[i++]); });
+
+    phrase_match matcher(strings, lengths, true);
+
+    auto run = [&matcher](const char *str, const char *expect) {
+        ddwaf_object param;
+        ddwaf_object_string(&param, str);
+        if (expect != nullptr) {
+            auto [res, highlight] = matcher.match(param);
+            EXPECT_TRUE(res);
+            EXPECT_STREQ(highlight.c_str(), expect);
+        } else {
+            EXPECT_FALSE(matcher.match(param).first);
+        }
+        ddwaf_object_free(&param);
+    };
+
+    run("banana", "banana");
+    run(" banana", "banana");
+    run("banana ", "banana");
+    run("word banana word", "banana");
+    run("word   ;banana/ word", "banana");
+
+    run("banan", nullptr);
+    run("abanana", nullptr);
+    run("bananaa", nullptr);
+    run("abananaa", nullptr);
+    run("banana_", nullptr);
+    run("_banana", nullptr);
+    run("_banana_", nullptr);
+    run("   _banana   ", nullptr);
+    run("   banana_   ", nullptr);
+    run("   _banana_   ", nullptr);
+
+    run("$apple", "$apple");
+    run("s$apple", "$apple");
+    run(";$apple", "$apple");
+    run(";$apple;", "$apple");
+    run("$apple;", "$apple");
+    run("word $apple word", "$apple");
+
+    run("apple", nullptr);
+    run("$applea", nullptr);
+    run("a$applea", nullptr);
+    run("$apple_", nullptr);
+    run("_$apple_", nullptr);
+    run("   $apple_   ", nullptr);
+    run("   _$apple_   ", nullptr);
+
+    run("orange$", "orange$");
+    run("orange$s", "orange$");
+    run(";orange$", "orange$");
+    run(";orange$;", "orange$");
+    run("orange$;", "orange$");
+    run("word orange$word", "orange$");
+
+    run("orange", nullptr);
+    run("aorange$", nullptr);
+    run("aorange$a", nullptr);
+    run("_orange$", nullptr);
+    run("_orange$_", nullptr);
+    run("   _orange$   ", nullptr);
+    run("   _orange$_   ", nullptr);
+
+    run("$pear$", "$pear$");
+    run("$pear$s", "$pear$");
+    run("s$pear$", "$pear$");
+    run("s$pear$s", "$pear$");
+    run(";$pear$", "$pear$");
+    run(";$pear$;", "$pear$");
+    run("$pear$;", "$pear$");
+    run("word$pear$word", "$pear$");
+    run("word $pear$ word", "$pear$");
+
+    run("pear$", nullptr);
+    run("$pear", nullptr);
+}
+
 TEST(TestPhraseMatch, TestInvalidInput)
 {
     std::vector<const char *> strings{"aaaa", "bbbb", "cccc"};
diff --git a/tests/utils_test.cpp b/tests/utils_test.cpp
@@ -90,6 +90,17 @@ TEST(TestUtils, IsAlnum)
     }
 }
 
+TEST(TestUtils, IsBoundary)
+{
+    for (char c = char_min; c < char_max; ++c) {
+        if ((c >= '0' && c <= '9') || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') ||
+            c == '_') {
+            EXPECT_FALSE(isboundary(c));
+        } else {
+            EXPECT_TRUE(isboundary(c));
+        }
+    }
+}
 TEST(TestUtils, ToLower)
 {
     std::unordered_map<char, char> mapping{{'A', 'a'}, {'B', 'b'}, {'C', 'c'}, {'D', 'd'},
diff --git a/validator/tests/rules/operators/phrase_match/001_rule1_pm_match.yaml b/validator/tests/rules/operators/phrase_match/001_rule1_pm_match.yaml
diff --git a/validator/tests/rules/operators/phrase_match/002_rule1_pm_no_match.yaml b/validator/tests/rules/operators/phrase_match/002_rule1_pm_no_match.yaml
diff --git a/validator/tests/rules/operators/phrase_match/003_rule2_pm_word_bound_match.yaml b/validator/tests/rules/operators/phrase_match/003_rule2_pm_word_bound_match.yaml
diff --git a/validator/tests/rules/operators/phrase_match/004_rule2_pm_word_bound_no_match.yaml b/validator/tests/rules/operators/phrase_match/004_rule2_pm_word_bound_no_match.yaml
diff --git a/validator/tests/rules/operators/phrase_match/ruleset.yaml b/validator/tests/rules/operators/phrase_match/ruleset.yaml

Original file line number	Diff line number	Diff line change
`@@ -112,6 +112,7 @@ inline bool isspace(char c)`
`112`	`112`	`inline bool isupper(char c) { return static_cast<unsigned>(c) - 'A' < 26; }`
`113`	`113`	`inline bool islower(char c) { return static_cast<unsigned>(c) - 'a' < 26; }`
`114`	`114`	`inline bool isalnum(char c) { return isalpha(c) \|\| isdigit(c); }`
	`115`	`+inline bool isboundary(char c) { return !isalnum(c) && c != '_'; }`
`115`	`116`	`inline char tolower(char c) { return isupper(c) ? static_cast<char>(c \| 32) : c; }`
`116`	`117`	`inline uint8_t from_hex(char c)`
`117`	`118`	`{`
Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,17 @@ TEST(TestUtils, IsAlnum)`
`90`	`90`	`}`
`91`	`91`	`}`
`92`	`92`
	`93`	`+TEST(TestUtils, IsBoundary)`
	`94`	`+{`
	`95`	`+ for (char c = char_min; c < char_max; ++c) {`
	`96`	`+ if ((c >= '0' && c <= '9') \|\| (c >= 'A' && c <= 'Z') \|\| (c >= 'a' && c <= 'z') \|\|`
	`97`	`+ c == '_') {`
	`98`	`+ EXPECT_FALSE(isboundary(c));`
	`99`	`+ } else {`
	`100`	`+ EXPECT_TRUE(isboundary(c));`
	`101`	`+ }`
	`102`	`+ }`
	`103`	`+}`
`93`	`104`	`TEST(TestUtils, ToLower)`
`94`	`105`	`{`
`95`	`106`	`std::unordered_map<char, char> mapping{{'A', 'a'}, {'B', 'b'}, {'C', 'c'}, {'D', 'd'},`