feat: add opt-in build flag to match v2's @rx anchor semantics (#3295)

.gitignore

@@ -36,6 +36,7 @@ src/headers.mk
 /test/rules_optimization
 /test/regression_tests
 /test/unit_tests
+/test/regex_multiline_test
 /test-driver
 /test/massif.out.*
 /test/benchmark/benchmark

README.md

@@ -115,6 +115,16 @@ Windows build information can be found [here](build/win32/README.md).
   * **Default:** PCRE2 is detected and used.
   * **Fallback:** legacy PCRE can be used if `--with-pcre` is explicitly provided (`WITH_PCRE`).
 * In other words, current builds expect PCRE2 unless explicitly configured otherwise.
+* `@rx` and `@rxGlobal` compile patterns with `DOTALL | MULTILINE` by default: `DOTALL`
+  lets `.` match newlines, and `MULTILINE` lets `^`/`$` anchor at every internal line
+  break rather than only at the start/end of the whole subject. Pass
+  `--enable-regex-dollar-endonly=yes` to compile them with `DOTALL | DOLLAR_ENDONLY`
+  instead: without `MULTILINE`, `^`/`$` anchor only at the start/end of the whole
+  subject, and `DOLLAR_ENDONLY` further restricts `$` to match only there rather than
+  also just before a trailing newline. `DOTALL` is unchanged either way, so patterns
+  can still match across internal line breaks via `.`. This matches ModSecurity v2's
+  `@rx` behavior. The flag is disabled by default; see
+  [issue #3295](https://github.com/owasp-modsecurity/ModSecurity/issues/3295).
 
 All other dependencies are related to operators specified within SecRules or configuration directives and may not be required for compilation.
 

configure.ac

@@ -285,6 +285,11 @@ MSC_ARG_ENABLE_BOOL([afl-fuzz], [Turn on the afl fuzzer compilation utilities],
 MSC_ARG_ENABLE_BOOL([examples], [Turn on the examples compilation (default option)], [true], [buildExamples])
 MSC_ARG_ENABLE_BOOL([parser-generation], [Enables parser generation during the build], [false], [buildParser])
 MSC_ARG_ENABLE_BOOL([mutex-on-pm], [Treat pm operations as critical section], [false], [mutexPm])
+MSC_ARG_ENABLE_BOOL([regex-dollar-endonly], [Compile @rx/@rxGlobal patterns with PCRE(2)_DOLLAR_ENDONLY instead of PCRE(2)_MULTILINE, matching ModSecurity v2 behavior (see issue #3295)], [false], [regexDollarEndonly])
+if test "$regexDollarEndonly" = "true"; then
+    MODSEC_REGEX_DOLLAR_ENDONLY_CPPFLAGS="-DMODSEC_REGEX_DOLLAR_ENDONLY=1"
+    GLOBAL_CPPFLAGS="$GLOBAL_CPPFLAGS $MODSEC_REGEX_DOLLAR_ENDONLY_CPPFLAGS"
+fi
 
 
 if test $buildParser = true; then

src/operators/rx.cc

@@ -26,10 +26,15 @@
 namespace modsecurity {
 namespace operators {
 
+#ifdef MODSEC_REGEX_DOLLAR_ENDONLY
+static const bool kRxMultiLine = false;
+#else
+static const bool kRxMultiLine = true;
+#endif
 
 bool Rx::init(const std::string &arg, std::string *error) {
     if (m_string->m_containsMacro == false) {
-        m_re = new Regex(m_param);
+        m_re = new Regex(m_param, false, kRxMultiLine);
     }
 
     return true;
@@ -46,7 +51,7 @@ bool Rx::evaluate(Transaction *transaction, RuleWithActions *rule,
 
     if (m_string->m_containsMacro) {
         std::string eparam(m_string->evaluate(transaction));
-        re = new Regex(eparam);
+        re = new Regex(eparam, false, kRxMultiLine);
     } else {
         re = m_re;
     }

src/operators/rx_global.cc

@@ -26,10 +26,15 @@
 namespace modsecurity {
 namespace operators {
 
+#ifdef MODSEC_REGEX_DOLLAR_ENDONLY
+static const bool kRxGlobalMultiLine = false;
+#else
+static const bool kRxGlobalMultiLine = true;
+#endif
 
 bool RxGlobal::init(const std::string &arg, std::string *error) {
     if (m_string->m_containsMacro == false) {
-        m_re = new Regex(m_param);
+        m_re = new Regex(m_param, false, kRxGlobalMultiLine);
     }
 
     return true;
@@ -46,7 +51,7 @@ bool RxGlobal::evaluate(Transaction *transaction, RuleWithActions *rule,
 
     if (m_string->m_containsMacro) {
         std::string eparam(m_string->evaluate(transaction));
-        re = new Regex(eparam);
+        re = new Regex(eparam, false, kRxGlobalMultiLine);
     } else {
         re = m_re;
     }

src/utils/regex.cc

@@ -87,11 +87,12 @@ bool crlfIsNewline() {
     return crlf_is_newline;
 }
 
-Regex::Regex(const std::string& pattern_, bool ignoreCase)
+Regex::Regex(const std::string& pattern_, bool ignoreCase, bool multiLine)
     : pattern(pattern_.empty() ? ".*" : pattern_) {
 #ifndef WITH_PCRE
     PCRE2_SPTR pcre2_pattern = reinterpret_cast<PCRE2_SPTR>(pattern.c_str());
-    uint32_t pcre2_options = (PCRE2_DOTALL|PCRE2_MULTILINE);
+    uint32_t pcre2_options = PCRE2_DOTALL |
+        (multiLine ? PCRE2_MULTILINE : PCRE2_DOLLAR_ENDONLY);
     if (ignoreCase) {
         pcre2_options |= PCRE2_CASELESS;
     }
@@ -103,7 +104,7 @@ Regex::Regex(const std::string& pattern_, bool ignoreCase)
 #else
     const char *errptr = nullptr;
     int erroffset;
-    int flags = (PCRE_DOTALL|PCRE_MULTILINE);
+    int flags = PCRE_DOTALL | (multiLine ? PCRE_MULTILINE : PCRE_DOLLAR_ENDONLY);
 
     if (ignoreCase == true) {
         flags |= PCRE_CASELESS;

src/utils/regex.h

@@ -71,7 +71,8 @@ struct SMatchCapture {
 
 class Regex {
  public:
-    explicit Regex(const std::string& pattern_, bool ignoreCase = false);
+    explicit Regex(const std::string& pattern_, bool ignoreCase = false,
+        bool multiLine = true);
     ~Regex();
 
     // m_pc and m_pce can't be easily copied

test/Makefile.am

@@ -70,6 +70,32 @@ unit_tests_LDFLAGS = \
 	$(YAJL_LDFLAGS)
 
 
+# regex_multiline_test
+#
+# Direct coverage for the Regex(pattern, ignoreCase, multiLine) parameter
+# behind --enable-regex-dollar-endonly, independent of the rest of the
+# SecRules pipeline exercised by unit_tests/regression_tests.
+
+noinst_PROGRAMS += regex_multiline_test
+regex_multiline_test_SOURCES = \
+        unit/regex_multiline_test.cc
+
+regex_multiline_test_LDFLAGS = \
+	-L$(top_builddir)/src/.libs/ \
+	-lmodsecurity \
+	-lpthread \
+	-lm \
+	-lstdc++
+
+regex_multiline_test_CPPFLAGS = \
+	-I$(top_srcdir)/ \
+	-g \
+	-I$(top_builddir)/headers \
+	$(GLOBAL_CPPFLAGS) \
+	$(PCRE_CFLAGS) \
+	$(PCRE2_CFLAGS)
+
+
 unit_tests_CPPFLAGS = \
 	-Icommon \
 	-I$(top_srcdir)/ \

test/test-suite.in

@@ -1,4 +1,5 @@
 # for i in `find test/test-cases -iname *.json`; do echo TESTS+=$i; done
+TESTS+=test/unit/regex_multiline_test
 TESTS+=test/test-cases/regression/action-allow.json
 TESTS+=test/test-cases/regression/action-block.json
 TESTS+=test/test-cases/regression/action-ctl_request_body_access.json

test/test-suite.sh

@@ -8,7 +8,15 @@
 PARAM=$array
 FILE=${@: -1}
 
-if [[ $FILE == *"test-cases/regression/"* ]]
+if [[ $FILE != *.json ]]
+then
+    # Self-contained test binary (not a JSON test-case file) - run it directly.
+    $VALGRIND $PARAM ./$(basename $FILE)
+    RET=$?
+    if [ $RET -ne 0 ]; then
+        echo ":test-result: FAIL: ../$FILE"
+    fi
+elif [[ $FILE == *"test-cases/regression/"* ]]
 then
     AMOUNT=$(./regression_tests countall ../$FILE)
     RET=$?
@@ -20,7 +28,7 @@
     for i in `seq 1 $AMOUNT`; do
         $VALGRIND $PARAM ./regression_tests ../$FILE:$i
         RET=$?
         if [ $RET -ne 0 ]; then
             echo ":test-result: FAIL possible segfault/$RET: ../$FILE:$i"
         fi
         echo $VALGRIND $PARAM ./regression_tests ../$FILE:$i

test/unit/regex_multiline_test.cc

@@ -0,0 +1,82 @@
+/*
+ * ModSecurity, http://www.modsecurity.org/
+ * Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *
+ * You may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * If any of the files related to licensing are missing or if you have any
+ * other questions related to licensing please contact Trustwave Holdings, Inc.
+ * directly using the email address security@modsecurity.org.
+ *
+ */
+
+#include <iostream>
+#include <string>
+
+#include "src/utils/regex.h"
+
+using modsecurity::Utils::Regex;
+
+namespace {
+
+int g_failures = 0;
+
+void check(const std::string &label, const Regex &re, const std::string &input,
+    bool expectMatch) {
+    const bool matched = re.search(input) > 0;
+    if (matched != expectMatch) {
+        std::cerr << "FAIL: " << label << " - expected "
+            << (expectMatch ? "match" : "no match") << ", got "
+            << (matched ? "match" : "no match") << std::endl;
+        g_failures++;
+    } else {
+        std::cout << "PASS: " << label << std::endl;
+    }
+}
+
+}  // namespace
+
+/*
+ * Regression coverage for the Regex(pattern, ignoreCase, multiLine) parameter
+ * added to support --enable-regex-dollar-endonly (see README.md and
+ * https://github.com/owasp-modsecurity/ModSecurity/issues/3295). Every other
+ * Regex call site in the codebase relies on the multiLine=true default and is
+ * unaffected by this parameter.
+ */
+int main() {
+    const std::string pattern = "^hello.*world$";
+    const std::string multiLineInput = "test\nhello world\nmore";
+
+    /* multiLine=true (default): PCRE2_MULTILINE / PCRE_MULTILINE, so ^/$
+     * anchor at internal line breaks and the pattern matches the middle
+     * line on its own. */
+    check("multiLine=true matches across internal line breaks",
+        Regex(pattern, false, true), multiLineInput, true);
+
+    /* multiLine=false: PCRE2_DOLLAR_ENDONLY / PCRE_DOLLAR_ENDONLY, so ^/$
+     * anchor only at the start/end of the whole subject, matching
+     * ModSecurity v2's @rx behavior. */
+    check("multiLine=false does not match across internal line breaks",
+        Regex(pattern, false, false), multiLineInput, false);
+
+    check("multiLine=false still matches a single-line subject",
+        Regex(pattern, false, false), "hello world", true);
+
+    /* DOLLAR_ENDONLY specifically makes '$' match only at the true end of
+     * the subject, not just before a trailing newline. */
+    check("multiLine=false rejects a trailing newline before '$'",
+        Regex("123$", false, false), "123\n", false);
+    check("multiLine=false matches '$' at the true end of the subject",
+        Regex("123$", false, false), "123", true);
+
+    if (g_failures == 0) {
+        std::cout << "All Regex multiLine tests passed." << std::endl;
+    } else {
+        std::cout << g_failures << " Regex multiLine test(s) failed." << std::endl;
+    }
+
+    return g_failures;
+}