From e2eff2b52a845ff527cf514bdbd3ecfed34975d2 Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Sun, 27 Sep 2026 00:30:16 -0300 Subject: [PATCH 1/7] Fix escaping of request-derived RuleMessage fields --- src/operators/operator.cc | 13 ++-- src/rule_message.cc | 24 ++++-- test/test-cases/regression/issue-3601.json | 90 ++++++++++++++++++++++ test/test-suite.in | 1 + 4 files changed, 115 insertions(+), 13 deletions(-) create mode 100644 test/test-cases/regression/issue-3601.json diff --git a/src/operators/operator.cc b/src/operators/operator.cc index 935add6211..d0dde069af 100644 --- a/src/operators/operator.cc +++ b/src/operators/operator.cc @@ -112,17 +112,20 @@ std::string Operator::resolveMatchMessage(Transaction *t, if (m_couldContainsMacro == false) { ret = "Matched \"Operator `" + m_op + "' with parameter `" + utils::string::limitTo(200, m_param) + - "' against variable `" + utils::string::toHexIfNeeded(key) + "' (Value: `" + + "' against variable `" + + utils::string::toHexIfNeeded(key, true) + "' (Value: `" + utils::string::limitTo(100, - utils::string::toHexIfNeeded(value)) + \ + utils::string::toHexIfNeeded(value, true)) + \ "' )"; } else { std::string p(m_string->evaluate(t)); ret = "Matched \"Operator `" + m_op + "' with parameter `" + - utils::string::limitTo(200, p) + - "' against variable `" + utils::string::toHexIfNeeded(key) + "' (Value: `" + + utils::string::toHexIfNeeded( + utils::string::limitTo(200, p), true) + + "' against variable `" + + utils::string::toHexIfNeeded(key, true) + "' (Value: `" + utils::string::limitTo(100, - utils::string::toHexIfNeeded(value)) + + utils::string::toHexIfNeeded(value, true)) + "' )"; } } diff --git a/src/rule_message.cc b/src/rule_message.cc index 7c021d17eb..187a762cee 100644 --- a/src/rule_message.cc +++ b/src/rule_message.cc @@ -30,7 +30,8 @@ std::string RuleMessage::_details(const RuleMessage &rm) { msg.append(" [line \"" + std::to_string(rm.m_rule.getLineNumber()) + "\"]"); msg.append(" [id \"" + std::to_string(rm.m_rule.m_ruleId) + "\"]"); msg.append(" [rev \"" + utils::string::toHexIfNeeded(rm.m_rule.m_rev, true) + "\"]"); - msg.append(" [msg \"" + rm.m_message + "\"]"); + msg.append(" [msg \"" + + utils::string::toHexIfNeeded(rm.m_message, true) + "\"]"); msg.append(" [data \"" + utils::string::toHexIfNeeded(utils::string::limitTo(200, rm.m_data), true) + "\"]"); msg.append(" [severity \"" + std::to_string(rm.m_severity) + "\"]"); @@ -42,10 +43,13 @@ std::string RuleMessage::_details(const RuleMessage &rm) { msg.append(" [tag \"" + utils::string::toHexIfNeeded(a, true) + "\"]"); } - msg.append(" [hostname \"" + rm.m_transaction.m_requestHostName \ - + "\"]"); - msg.append(" [uri \"" + utils::string::limitTo(200, rm.m_transaction.m_uri_no_query_string_decoded) + "\"]"); - msg.append(" [unique_id \"" + rm.m_transaction.m_id + "\"]"); + msg.append(" [hostname \"" + utils::string::toHexIfNeeded( + rm.m_transaction.m_requestHostName, true) + "\"]"); + msg.append(" [uri \"" + utils::string::toHexIfNeeded( + utils::string::limitTo(200, + rm.m_transaction.m_uri_no_query_string_decoded), true) + "\"]"); + msg.append(" [unique_id \"" + utils::string::toHexIfNeeded( + rm.m_transaction.m_id, true) + "\"]"); msg.append(" [ref \"" + utils::string::limitTo(200, rm.m_reference) + "\"]"); return msg; @@ -55,9 +59,13 @@ std::string RuleMessage::_details(const RuleMessage &rm) { std::string RuleMessage::_errorLogTail(const RuleMessage &rm) { std::string msg; - msg.append("[hostname \"" + rm.m_transaction.m_serverIpAddress + "\"]"); - msg.append(" [uri \"" + utils::string::limitTo(200, rm.m_transaction.m_uri_no_query_string_decoded) + "\"]"); - msg.append(" [unique_id \"" + rm.m_transaction.m_id + "\"]"); + msg.append("[hostname \"" + utils::string::toHexIfNeeded( + rm.m_transaction.m_serverIpAddress, true) + "\"]"); + msg.append(" [uri \"" + utils::string::toHexIfNeeded( + utils::string::limitTo(200, + rm.m_transaction.m_uri_no_query_string_decoded), true) + "\"]"); + msg.append(" [unique_id \"" + utils::string::toHexIfNeeded( + rm.m_transaction.m_id, true) + "\"]"); return msg; } diff --git a/test/test-cases/regression/issue-3601.json b/test/test-cases/regression/issue-3601.json new file mode 100644 index 0000000000..ae0f60026d --- /dev/null +++ b/test/test-cases/regression/issue-3601.json @@ -0,0 +1,90 @@ +[ + { + "enabled": 1, + "version_min": 300000, + "title": "RuleMessage text log preserves normal field delimiters", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "ordinary", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "error_log": "\\Q[msg \"request=ordinary\"] [data \"\"] [severity \"0\"] [ver \"\"] [maturity \"0\"] [accuracy \"0\"] [hostname \"normal.example\"] [uri \"/normal\"]\\E", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_URI \"@contains /normal\" \"id:3601001,phase:1,pass,log,msg:'request=%{REQUEST_HEADERS:X-Log}'\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "RuleMessage text log escapes request-derived field delimiters", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "host.example\" ] [uri \"forged\\literal\\x22\r\nend" + }, + "request": { + "headers": { + "Host": "request.example", + "X-Log": "message\" ] [hostname \"forged\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/safe\" ] [unique_id \"forged\\literal\\x22\r\nend", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "error_log": "(?=.*\\Q(Value: `/safe\\x22 ] [unique_id \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend' )\\E)(?=.*\\Q[msg \"request=message\\x22 ] [hostname \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"] [data \"\"] [severity \"0\"] [ver \"\"] [maturity \"0\"] [accuracy \"0\"] [hostname \"host.example\\x22 ] [uri \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"] [uri \"/safe\\x22 ] [unique_id \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"]\\E)", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_URI \"@contains /safe\" \"id:3601002,phase:1,pass,log,msg:'request=%{REQUEST_HEADERS:X-Log}'\"" + ] + } +] diff --git a/test/test-suite.in b/test/test-suite.in index ebda49fb81..88a7b25668 100644 --- a/test/test-suite.in +++ b/test/test-suite.in @@ -74,6 +74,7 @@ TESTS+=test/test-cases/regression/issue-2423-msg-in-chain.json TESTS+=test/test-cases/regression/issue-2427.json TESTS+=test/test-cases/regression/issue-2296.json TESTS+=test/test-cases/regression/issue-3340.json +TESTS+=test/test-cases/regression/issue-3601.json TESTS+=test/test-cases/regression/issue-394.json TESTS+=test/test-cases/regression/issue-849.json TESTS+=test/test-cases/regression/issue-960.json From 5e5584810c038d4ad7528f6990981db7bbe6e9ea Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Sun, 27 Sep 2026 10:44:10 -0300 Subject: [PATCH 2/7] Make log message escaping opt-in --- .github/workflows/ci.yml | 5 ++ .github/workflows/ci_new.yml | 5 ++ configure.ac | 15 ++++ src/operators/operator.cc | 21 +++-- src/rule_message.cc | 24 +++-- .../regression/issue-3601-legacy.json | 90 +++++++++++++++++++ test/test-suite.in | 4 + 7 files changed, 152 insertions(+), 12 deletions(-) create mode 100644 test/test-cases/regression/issue-3601-legacy.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9add866e42..be327115bc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,11 +28,16 @@ jobs: - {label: "wo ssdeep", opt: "--without-ssdeep" } - {label: "with lmdb", opt: "--with-lmdb" } - {label: "with pcre", opt: "--with-pcre" } + - {label: "with log message escape", opt: "--enable-log-message-escape" } exclude: - platform: {label: "x32"} configure: {label: "wo geoip"} - platform: {label: "x32"} configure: {label: "wo ssdeep"} + - platform: {label: "x32"} + configure: {label: "with log message escape"} + - compiler: {label: "clang"} + configure: {label: "with log message escape"} steps: - name: Detect latest Lua dev package id: detect_lua diff --git a/.github/workflows/ci_new.yml b/.github/workflows/ci_new.yml index fd0845243b..4382eb19c3 100644 --- a/.github/workflows/ci_new.yml +++ b/.github/workflows/ci_new.yml @@ -33,6 +33,11 @@ jobs: - { label: "with lmdb", opt: "--with-lmdb" } - { label: "with pcre2 (default)", opt: "" } - { label: "with pcre", opt: "--with-pcre" } + - { label: "with log message escape", opt: "--enable-log-message-escape" } + + exclude: + - compiler: { label: "clang" } + configure: { label: "with log message escape" } steps: - uses: actions/checkout@v6 diff --git a/configure.ac b/configure.ac index 7bdcca6bc4..33316dae9c 100644 --- a/configure.ac +++ b/configure.ac @@ -276,11 +276,19 @@ AC_SUBST([MSC_GIT_VERSION]) MSC_ARG_ENABLE_BOOL([assertions], [Turn on assertions feature: undefine NDEBUG], [false], [assertions]) MSC_ARG_ENABLE_BOOL([debug-logs], [Turn off the SecDebugLog feature], [true], [debugLogs]) +MSC_ARG_ENABLE_BOOL([log-message-escape], + [Escape quotes and backslashes in textual log message fields], + [false], [logMessageEscape]) if test "$debugLogs" != "true"; then MODSEC_NO_LOGS="-DNO_LOGS=1" AC_SUBST(MODSEC_NO_LOGS) fi +if test "$logMessageEscape" = "true"; then + AC_DEFINE([MSC_LOG_MESSAGE_ESCAPE], [1], + [Define to escape quotes and backslashes in textual log message fields]) +fi + MSC_ARG_ENABLE_BOOL([afl-fuzz], [Turn on the afl fuzzer compilation utilities], [false], [aflFuzzer]) 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]) @@ -328,6 +336,7 @@ fi AM_CONDITIONAL([EXAMPLES], [test $buildExamples = true]) AM_CONDITIONAL([BUILD_PARSER], [test $buildParser = true]) AM_CONDITIONAL([USE_MUTEX_ON_PM], [test $mutexPm = true]) +AM_CONDITIONAL([LOG_MESSAGE_ESCAPE], [test $logMessageEscape = true]) # General link options @@ -492,6 +501,12 @@ else echo " + SecDebugLog ....disabled" fi +if test "$logMessageEscape" = "true"; then + echo " + Log message escape ....enabled" +else + echo " + Log message escape ....disabled" +fi + if test "$aflFuzzer" = "true"; then echo " + afl fuzzer ....enabled" echo " ($FUZZ_CPPCFLAGS)" diff --git a/src/operators/operator.cc b/src/operators/operator.cc index d0dde069af..c7ec67aa88 100644 --- a/src/operators/operator.cc +++ b/src/operators/operator.cc @@ -20,6 +20,7 @@ #include #include "modsecurity/transaction.h" +#include "src/config.h" #include "src/run_time_string.h" #include "src/utils/string.h" #include "src/operators/begins_with.h" @@ -66,6 +67,12 @@ #define IF_MATCH(a) \ if (op_ == #a) +#ifdef MSC_LOG_MESSAGE_ESCAPE +constexpr bool kEscapeLogMessage = true; +#else +constexpr bool kEscapeLogMessage = false; +#endif + namespace modsecurity { namespace operators { @@ -113,19 +120,23 @@ std::string Operator::resolveMatchMessage(Transaction *t, ret = "Matched \"Operator `" + m_op + "' with parameter `" + utils::string::limitTo(200, m_param) + "' against variable `" + - utils::string::toHexIfNeeded(key, true) + "' (Value: `" + + utils::string::toHexIfNeeded(key, kEscapeLogMessage) + + "' (Value: `" + utils::string::limitTo(100, - utils::string::toHexIfNeeded(value, true)) + \ + utils::string::toHexIfNeeded( + value, kEscapeLogMessage)) + \ "' )"; } else { std::string p(m_string->evaluate(t)); ret = "Matched \"Operator `" + m_op + "' with parameter `" + utils::string::toHexIfNeeded( - utils::string::limitTo(200, p), true) + + utils::string::limitTo(200, p), kEscapeLogMessage) + "' against variable `" + - utils::string::toHexIfNeeded(key, true) + "' (Value: `" + + utils::string::toHexIfNeeded(key, kEscapeLogMessage) + + "' (Value: `" + utils::string::limitTo(100, - utils::string::toHexIfNeeded(value, true)) + + utils::string::toHexIfNeeded( + value, kEscapeLogMessage)) + "' )"; } } diff --git a/src/rule_message.cc b/src/rule_message.cc index 187a762cee..0803b739e4 100644 --- a/src/rule_message.cc +++ b/src/rule_message.cc @@ -18,8 +18,15 @@ #include "modsecurity/rules_set.h" #include "modsecurity/modsecurity.h" #include "modsecurity/transaction.h" +#include "src/config.h" #include "src/utils/string.h" +#ifdef MSC_LOG_MESSAGE_ESCAPE +constexpr bool kEscapeLogMessage = true; +#else +constexpr bool kEscapeLogMessage = false; +#endif + namespace modsecurity { @@ -31,7 +38,8 @@ std::string RuleMessage::_details(const RuleMessage &rm) { msg.append(" [id \"" + std::to_string(rm.m_rule.m_ruleId) + "\"]"); msg.append(" [rev \"" + utils::string::toHexIfNeeded(rm.m_rule.m_rev, true) + "\"]"); msg.append(" [msg \"" + - utils::string::toHexIfNeeded(rm.m_message, true) + "\"]"); + utils::string::toHexIfNeeded( + rm.m_message, kEscapeLogMessage) + "\"]"); msg.append(" [data \"" + utils::string::toHexIfNeeded(utils::string::limitTo(200, rm.m_data), true) + "\"]"); msg.append(" [severity \"" + std::to_string(rm.m_severity) + "\"]"); @@ -44,12 +52,13 @@ std::string RuleMessage::_details(const RuleMessage &rm) { } msg.append(" [hostname \"" + utils::string::toHexIfNeeded( - rm.m_transaction.m_requestHostName, true) + "\"]"); + rm.m_transaction.m_requestHostName, kEscapeLogMessage) + "\"]"); msg.append(" [uri \"" + utils::string::toHexIfNeeded( utils::string::limitTo(200, - rm.m_transaction.m_uri_no_query_string_decoded), true) + "\"]"); + rm.m_transaction.m_uri_no_query_string_decoded), + kEscapeLogMessage) + "\"]"); msg.append(" [unique_id \"" + utils::string::toHexIfNeeded( - rm.m_transaction.m_id, true) + "\"]"); + rm.m_transaction.m_id, kEscapeLogMessage) + "\"]"); msg.append(" [ref \"" + utils::string::limitTo(200, rm.m_reference) + "\"]"); return msg; @@ -60,12 +69,13 @@ std::string RuleMessage::_errorLogTail(const RuleMessage &rm) { std::string msg; msg.append("[hostname \"" + utils::string::toHexIfNeeded( - rm.m_transaction.m_serverIpAddress, true) + "\"]"); + rm.m_transaction.m_serverIpAddress, kEscapeLogMessage) + "\"]"); msg.append(" [uri \"" + utils::string::toHexIfNeeded( utils::string::limitTo(200, - rm.m_transaction.m_uri_no_query_string_decoded), true) + "\"]"); + rm.m_transaction.m_uri_no_query_string_decoded), + kEscapeLogMessage) + "\"]"); msg.append(" [unique_id \"" + utils::string::toHexIfNeeded( - rm.m_transaction.m_id, true) + "\"]"); + rm.m_transaction.m_id, kEscapeLogMessage) + "\"]"); return msg; } diff --git a/test/test-cases/regression/issue-3601-legacy.json b/test/test-cases/regression/issue-3601-legacy.json new file mode 100644 index 0000000000..5c7ebdc860 --- /dev/null +++ b/test/test-cases/regression/issue-3601-legacy.json @@ -0,0 +1,90 @@ +[ + { + "enabled": 1, + "version_min": 300000, + "title": "RuleMessage text log preserves normal field delimiters with legacy escaping", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "ordinary", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "error_log": "\\Q[msg \"request=ordinary\"] [data \"\"] [severity \"0\"] [ver \"\"] [maturity \"0\"] [accuracy \"0\"] [hostname \"normal.example\"] [uri \"/normal\"]\\E", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_URI \"@contains /normal\" \"id:3601001,phase:1,pass,log,msg:'request=%{REQUEST_HEADERS:X-Log}'\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "RuleMessage text log retains legacy special-character escaping", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "host.example\" ] [uri \"forged\\literal\\x22\r\nend" + }, + "request": { + "headers": { + "Host": "request.example", + "X-Log": "message\" ] [hostname \"forged\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/safe\" ] [unique_id \"forged\\literal\\x22\r\nend", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "error_log": "(?=.*\\Q(Value: `/safe\" ] [unique_id \"forged\\literal\\x22\\x0d\\x0aend' )\\E)(?=.*\\Q[msg \"request=message\" ] [hostname \"forged\\literal\\x22\\x0d\\x0aend\"] [data \"\"] [severity \"0\"] [ver \"\"] [maturity \"0\"] [accuracy \"0\"] [hostname \"host.example\" ] [uri \"forged\\literal\\x22\\x0d\\x0aend\"] [uri \"/safe\" ] [unique_id \"forged\\literal\\x22\\x0d\\x0aend\"]\\E)", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_URI \"@contains /safe\" \"id:3601002,phase:1,pass,log,msg:'request=%{REQUEST_HEADERS:X-Log}'\"" + ] + } +] diff --git a/test/test-suite.in b/test/test-suite.in index 88a7b25668..7be1993bdb 100644 --- a/test/test-suite.in +++ b/test/test-suite.in @@ -74,7 +74,11 @@ TESTS+=test/test-cases/regression/issue-2423-msg-in-chain.json TESTS+=test/test-cases/regression/issue-2427.json TESTS+=test/test-cases/regression/issue-2296.json TESTS+=test/test-cases/regression/issue-3340.json +if LOG_MESSAGE_ESCAPE TESTS+=test/test-cases/regression/issue-3601.json +else +TESTS+=test/test-cases/regression/issue-3601-legacy.json +endif TESTS+=test/test-cases/regression/issue-394.json TESTS+=test/test-cases/regression/issue-849.json TESTS+=test/test-cases/regression/issue-960.json From 8e8618fad7871ecd8e740b6d5e225d522c94a61b Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Mon, 28 Sep 2026 14:13:37 -0300 Subject: [PATCH 3/7] Support log message escaping in CMake --- .github/workflows/ci.yml | 1 + .github/workflows/ci_new.yml | 1 + build/win32/CMakeLists.txt | 29 +++++++++++++++++++++++++++++ build/win32/config.h.cmake | 5 ++++- 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index be327115bc..dbd2b196b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -176,6 +176,7 @@ jobs: - {label: "wo maxmind", opt: "-DWITH_MAXMIND=OFF" } - {label: "wo libxml", opt: "-DWITH_LIBXML2=OFF" } - {label: "with lmdb", opt: "-DWITH_LMDB=ON" } + - {label: "with log message escape", opt: "-DLOG_MESSAGE_ESCAPE=ON" } steps: - uses: actions/checkout@v6 with: diff --git a/.github/workflows/ci_new.yml b/.github/workflows/ci_new.yml index 4382eb19c3..2db6bb9259 100644 --- a/.github/workflows/ci_new.yml +++ b/.github/workflows/ci_new.yml @@ -192,6 +192,7 @@ jobs: - { label: "without maxmind", opt: "-DWITH_MAXMIND=OFF" } - { label: "without libxml", opt: "-DWITH_LIBXML2=OFF" } - { label: "with lmdb", opt: "-DWITH_LMDB=ON" } + - { label: "with log message escape", opt: "-DLOG_MESSAGE_ESCAPE=ON" } steps: - uses: actions/checkout@v6 diff --git a/build/win32/CMakeLists.txt b/build/win32/CMakeLists.txt index 399d4727c1..fad7c38c1e 100644 --- a/build/win32/CMakeLists.txt +++ b/build/win32/CMakeLists.txt @@ -7,6 +7,8 @@ option(WITH_LUA "Include LUA support" ON) option(WITH_LIBXML2 "Include LibXML2 support" ON) option(WITH_MAXMIND "Include MaxMind support" ON) option(WITH_CURL "Include CURL support" ON) +option(LOG_MESSAGE_ESCAPE + "Escape quotes and backslashes in textual log message fields" OFF) option(USE_ASAN "Build with Address Sanitizer" OFF) @@ -126,6 +128,7 @@ enable_feature(HAVE_LUA ${WITH_LUA}) enable_feature(HAVE_LIBXML2 ${WITH_LIBXML2}) enable_feature(HAVE_MAXMIND ${WITH_MAXMIND}) enable_feature(HAVE_CURL ${WITH_CURL}) +enable_feature(MSC_LOG_MESSAGE_ESCAPE ${LOG_MESSAGE_ESCAPE}) include(${CMAKE_CURRENT_LIST_DIR}/ConfigureChecks.cmake) @@ -218,7 +221,33 @@ enable_testing() file(READ ${BASE_DIR}/test/test-suite.in TEST_FILES_RAW) string(REPLACE "\n" ";" TEST_FILES ${TEST_FILES_RAW}) +set(SKIP_CONDITIONAL_TEST FALSE) foreach(TEST_FILE ${TEST_FILES}) + string(STRIP "${TEST_FILE}" TEST_FILE) + + # Select the appropriate branch of the Automake conditional shared by + # the Autotools and CMake test suites. + if(TEST_FILE STREQUAL "if LOG_MESSAGE_ESCAPE") + if(LOG_MESSAGE_ESCAPE) + set(SKIP_CONDITIONAL_TEST FALSE) + else() + set(SKIP_CONDITIONAL_TEST TRUE) + endif() + continue() + elseif(TEST_FILE STREQUAL "else") + if(SKIP_CONDITIONAL_TEST) + set(SKIP_CONDITIONAL_TEST FALSE) + else() + set(SKIP_CONDITIONAL_TEST TRUE) + endif() + continue() + elseif(TEST_FILE STREQUAL "endif") + set(SKIP_CONDITIONAL_TEST FALSE) + continue() + elseif(SKIP_CONDITIONAL_TEST) + continue() + endif() + # ignore comment lines string(FIND ${TEST_FILE} "#" is_comment) if(NOT is_comment EQUAL 0) diff --git a/build/win32/config.h.cmake b/build/win32/config.h.cmake index 2f6a73085e..53d802f54b 100644 --- a/build/win32/config.h.cmake +++ b/build/win32/config.h.cmake @@ -63,6 +63,9 @@ /* Define if libcurl is available */ #cmakedefine HAVE_CURL +/* Define to escape quotes and backslashes in textual log message fields */ +#cmakedefine MSC_LOG_MESSAGE_ESCAPE 1 + /* Name of package */ #define PACKAGE "@PACKAGE_NAME@" @@ -89,4 +92,4 @@ #cmakedefine STDC_HEADERS #endif -#endif // ndef MODSECURITY_CONFIG_H \ No newline at end of file +#endif // ndef MODSECURITY_CONFIG_H From 8c85dc4ca9c962eb0b5d7f0eaeb66728854d5a16 Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Mon, 28 Sep 2026 15:55:11 -0300 Subject: [PATCH 4/7] Fix log field escaping in operator match messages --- src/operators/operator.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/operators/operator.cc b/src/operators/operator.cc index c7ec67aa88..421b8a939d 100644 --- a/src/operators/operator.cc +++ b/src/operators/operator.cc @@ -122,9 +122,11 @@ std::string Operator::resolveMatchMessage(Transaction *t, "' against variable `" + utils::string::toHexIfNeeded(key, kEscapeLogMessage) + "' (Value: `" + - utils::string::limitTo(100, - utils::string::toHexIfNeeded( - value, kEscapeLogMessage)) + \ + (kEscapeLogMessage + ? utils::string::toHexIfNeeded( + utils::string::limitTo(100, value), true) + : utils::string::limitTo(100, + utils::string::toHexIfNeeded(value, false))) + \ "' )"; } else { std::string p(m_string->evaluate(t)); @@ -134,9 +136,11 @@ std::string Operator::resolveMatchMessage(Transaction *t, "' against variable `" + utils::string::toHexIfNeeded(key, kEscapeLogMessage) + "' (Value: `" + - utils::string::limitTo(100, - utils::string::toHexIfNeeded( - value, kEscapeLogMessage)) + + (kEscapeLogMessage + ? utils::string::toHexIfNeeded( + utils::string::limitTo(100, value), true) + : utils::string::limitTo(100, + utils::string::toHexIfNeeded(value, false))) + \ "' )"; } } From f9cf085d871878286b9917120a5fe4bb70a56721 Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Tue, 6 Oct 2026 16:48:01 -0300 Subject: [PATCH 5/7] Fix macro quote regression expectation for opt-in log escaping Accept the explicit legacy and hex-escaped message representations while requiring the complete msg field and suffix. Configuration-specific issue-3601 fixtures continue to verify which representation each build produces. --- .../regression/misc-escaped-quote-after-macro-expansion.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json b/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json index 21bdfa90d8..08ae6148e8 100644 --- a/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json +++ b/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json @@ -211,7 +211,7 @@ ] }, "expected": { - "error_log": "prefixGET\\\\\"suffix", + "error_log": "\\[msg \"prefixGET(?:\\\\\"|\\\\x5c\\\\x22)suffix\"\\]", "http_code": 200 }, "rules": [ From 8370bd1832f3a0a2f78efce93bc9c65cee12b6db Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Tue, 6 Oct 2026 21:03:28 -0300 Subject: [PATCH 6/7] Address review checks for opt-in log escaping --- .github/workflows/ci.yml | 6 +-- .github/workflows/ci_new.yml | 6 +-- src/operators/operator.cc | 11 +++++ src/rule_message.cc | 17 +++++++ .../regression/issue-3601-legacy.json | 44 +++++++++++++++++++ test/test-cases/regression/issue-3601.json | 44 +++++++++++++++++++ ...c-escaped-quote-after-macro-expansion.json | 44 ------------------- 7 files changed, 122 insertions(+), 50 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dbd2b196b9..8d113ad965 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -104,7 +104,7 @@ jobs: CC: ${{ matrix.compiler.cc }} CXX: ${{ matrix.compiler.cxx }} run: ./configure ${{ matrix.platform.configure }} ${{ matrix.configure.opt }} --enable-assertions=yes - - uses: ammaraskar/gcc-problem-matcher@master + - uses: ammaraskar/gcc-problem-matcher@cb2e3f949c41b7818628f35f8484a9b0acf90cf9 - name: make run: make -j `nproc` - name: check @@ -153,7 +153,7 @@ jobs: run: ./build.sh - name: configure run: ./configure ${{ matrix.configure.opt }} --enable-assertions=yes - - uses: ammaraskar/gcc-problem-matcher@master + - uses: ammaraskar/gcc-problem-matcher@cb2e3f949c41b7818628f35f8484a9b0acf90cf9 - name: make run: make -j `sysctl -n hw.logicalcpu` - name: check @@ -186,7 +186,7 @@ jobs: run: | pip3 install conan --upgrade conan profile detect - - uses: ammaraskar/msvc-problem-matcher@master + - uses: ammaraskar/msvc-problem-matcher@47189a176388253d16c6f0b79eee6274512b761a - name: Build ${{ matrix.configuration }} ${{ matrix.platform.arch }} ${{ matrix.configure.label }} shell: cmd run: vcbuild.bat ${{ matrix.configuration }} ${{ matrix.platform.arch }} NO_ASAN "${{ matrix.configure.opt }}" diff --git a/.github/workflows/ci_new.yml b/.github/workflows/ci_new.yml index 2db6bb9259..0a6c81a021 100644 --- a/.github/workflows/ci_new.yml +++ b/.github/workflows/ci_new.yml @@ -111,7 +111,7 @@ jobs: CXX: ${{ matrix.compiler.cxx }} run: ./configure ${{ matrix.platform.configure }} ${{ matrix.configure.opt }} --enable-assertions=yes - - uses: ammaraskar/gcc-problem-matcher@master + - uses: ammaraskar/gcc-problem-matcher@cb2e3f949c41b7818628f35f8484a9b0acf90cf9 - name: Compile run: make -j "$(nproc)" @@ -167,7 +167,7 @@ jobs: - name: Configure run: ./configure ${{ matrix.configure.opt }} --enable-assertions=yes - - uses: ammaraskar/gcc-problem-matcher@master + - uses: ammaraskar/gcc-problem-matcher@cb2e3f949c41b7818628f35f8484a9b0acf90cf9 - name: Compile run: make -j "$(sysctl -n hw.logicalcpu)" @@ -205,7 +205,7 @@ jobs: pip3 install conan --upgrade conan profile detect - - uses: ammaraskar/msvc-problem-matcher@master + - uses: ammaraskar/msvc-problem-matcher@47189a176388253d16c6f0b79eee6274512b761a - name: Build project shell: cmd diff --git a/src/operators/operator.cc b/src/operators/operator.cc index 421b8a939d..4d784ff010 100644 --- a/src/operators/operator.cc +++ b/src/operators/operator.cc @@ -111,6 +111,17 @@ bool Operator::evaluateInternal(Transaction *transaction, } +/** + * Returns the operator's custom match message, or builds the default message. + * MSC_LOG_MESSAGE_ESCAPE enables quote and backslash escaping in variable + * names, values and macro-expanded parameters. In that mode, values are + * limited before escaping; otherwise the legacy formatting order is retained. + * + * @param t Transaction used to evaluate macro-expanded operator parameters. + * @param key Matched variable name, including its collection. + * @param value Matched variable value before transformations. + * @return Custom or generated textual match description. + */ std::string Operator::resolveMatchMessage(Transaction *t, std::string key, std::string value) { std::string ret = m_match_message; diff --git a/src/rule_message.cc b/src/rule_message.cc index 0803b739e4..b3155ffd0e 100644 --- a/src/rule_message.cc +++ b/src/rule_message.cc @@ -30,6 +30,15 @@ constexpr bool kEscapeLogMessage = false; namespace modsecurity { +/** + * Builds the bracketed rule and transaction fields for a textual log message. + * With MSC_LOG_MESSAGE_ESCAPE, request-derived values escape quotes and + * backslashes before insertion so the field delimiters remain unchanged. + * URI and data limits are applied before escaping their values. + * + * @param rm Rule match and transaction whose details are formatted. + * @return Textual fields, including the leading space before the first field. + */ std::string RuleMessage::_details(const RuleMessage &rm) { std::string msg; @@ -65,6 +74,14 @@ std::string RuleMessage::_details(const RuleMessage &rm) { } +/** + * Builds the server-address, URI and transaction-ID fields for an error log. + * MSC_LOG_MESSAGE_ESCAPE enables quote and backslash escaping within values; + * the URI is limited before escaping to avoid cutting an escape sequence. + * + * @param rm Rule match providing the transaction details. + * @return Bracketed error-log tail with no leading space. + */ std::string RuleMessage::_errorLogTail(const RuleMessage &rm) { std::string msg; diff --git a/test/test-cases/regression/issue-3601-legacy.json b/test/test-cases/regression/issue-3601-legacy.json index 5c7ebdc860..abe4f96fd8 100644 --- a/test/test-cases/regression/issue-3601-legacy.json +++ b/test/test-cases/regression/issue-3601-legacy.json @@ -86,5 +86,49 @@ "SecRuleEngine On", "SecRule REQUEST_URI \"@contains /safe\" \"id:3601002,phase:1,pass,log,msg:'request=%{REQUEST_HEADERS:X-Log}'\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "msg (unquoted action value): escaped double quote immediately after a %{macro} does not truncate the action value", + "url": "https://github.com/owasp-modsecurity/ModSecurity/pull/3641#pullrequestreview", + "client": { + "ip": "200.249.12.31", + "port": 2313 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/test", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "error_log": "\\Q[msg \"prefixGET\\\"suffix\"]\\E", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_METHOD \"@unconditionalMatch\" \"phase:1,id:10007,log,pass,msg:prefix%{REQUEST_METHOD}\\\"suffix\"" + ] } ] diff --git a/test/test-cases/regression/issue-3601.json b/test/test-cases/regression/issue-3601.json index ae0f60026d..ebeeb4e46b 100644 --- a/test/test-cases/regression/issue-3601.json +++ b/test/test-cases/regression/issue-3601.json @@ -86,5 +86,49 @@ "SecRuleEngine On", "SecRule REQUEST_URI \"@contains /safe\" \"id:3601002,phase:1,pass,log,msg:'request=%{REQUEST_HEADERS:X-Log}'\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "msg (unquoted action value): escaped double quote immediately after a %{macro} does not truncate the action value", + "url": "https://github.com/owasp-modsecurity/ModSecurity/pull/3641#pullrequestreview", + "client": { + "ip": "200.249.12.31", + "port": 2313 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "0" + }, + "uri": "/test", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "error_log": "\\Q[msg \"prefixGET\\x5c\\x22suffix\"]\\E", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_METHOD \"@unconditionalMatch\" \"phase:1,id:10007,log,pass,msg:prefix%{REQUEST_METHOD}\\\"suffix\"" + ] } ] diff --git a/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json b/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json index 08ae6148e8..c3210c90ed 100644 --- a/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json +++ b/test/test-cases/regression/misc-escaped-quote-after-macro-expansion.json @@ -174,49 +174,5 @@ "SecRuleEngine On", "SecRule REQUEST_METHOD \"@unconditionalMatch\" \"phase:1,id:10006,log,pass,logdata:\"value=%{REQUEST_METHOD}\\\\\\\"end\"\"" ] - }, - { - "enabled": 1, - "version_min": 300000, - "title": "msg (unquoted action value): escaped double quote immediately after a %{macro} does not truncate the action value", - "url": "https://github.com/owasp-modsecurity/ModSecurity/pull/3641#pullrequestreview", - "client": { - "ip": "200.249.12.31", - "port": 2313 - }, - "server": { - "ip": "200.249.12.31", - "port": 80 - }, - "request": { - "headers": { - "Host": "localhost", - "User-Agent": "curl/7.38.0", - "Accept": "*/*", - "Content-Length": "0" - }, - "uri": "/test", - "method": "GET", - "http_version": 1.1, - "body": [ - "" - ] - }, - "response": { - "headers": { - "Content-Length": "0" - }, - "body": [ - "" - ] - }, - "expected": { - "error_log": "\\[msg \"prefixGET(?:\\\\\"|\\\\x5c\\\\x22)suffix\"\\]", - "http_code": 200 - }, - "rules": [ - "SecRuleEngine On", - "SecRule REQUEST_METHOD \"@unconditionalMatch\" \"phase:1,id:10007,log,pass,msg:prefix%{REQUEST_METHOD}\\\"suffix\"" - ] } ] From 57707aceede9bc0de3cd3a856cfff312e8853774 Mon Sep 17 00:00:00 2001 From: Guilherme Facini Date: Thu, 22 Oct 2026 12:06:40 -0300 Subject: [PATCH 7/7] Preserve JSON match values when escaping textual logs --- src/operators/operator.cc | 42 ++-- src/rule_message.cc | 20 +- test/test-cases/regression/issue-1528.json | 2 +- .../regression/issue-3601-legacy.json | 200 +++++++++++++++++ test/test-cases/regression/issue-3601.json | 202 +++++++++++++++++- test/test-cases/regression/operator-rx.json | 8 +- 6 files changed, 437 insertions(+), 37 deletions(-) diff --git a/src/operators/operator.cc b/src/operators/operator.cc index 4d784ff010..4162ca1de1 100644 --- a/src/operators/operator.cc +++ b/src/operators/operator.cc @@ -20,7 +20,6 @@ #include #include "modsecurity/transaction.h" -#include "src/config.h" #include "src/run_time_string.h" #include "src/utils/string.h" #include "src/operators/begins_with.h" @@ -67,12 +66,6 @@ #define IF_MATCH(a) \ if (op_ == #a) -#ifdef MSC_LOG_MESSAGE_ESCAPE -constexpr bool kEscapeLogMessage = true; -#else -constexpr bool kEscapeLogMessage = false; -#endif - namespace modsecurity { namespace operators { @@ -112,15 +105,15 @@ bool Operator::evaluateInternal(Transaction *transaction, /** - * Returns the operator's custom match message, or builds the default message. - * MSC_LOG_MESSAGE_ESCAPE enables quote and backslash escaping in variable - * names, values and macro-expanded parameters. In that mode, values are - * limited before escaping; otherwise the legacy formatting order is retained. + * Returns the operator's custom match message, or builds the legacy message. + * Keep this representation independent of textual log escaping: it is also + * serialized as the JSON audit log's match value. RuleMessage::log applies + * optional quote and backslash escaping when producing textual output. * * @param t Transaction used to evaluate macro-expanded operator parameters. * @param key Matched variable name, including its collection. * @param value Matched variable value before transformations. - * @return Custom or generated textual match description. + * @return Custom or generated match description in the legacy representation. */ std::string Operator::resolveMatchMessage(Transaction *t, std::string key, std::string value) { @@ -130,28 +123,17 @@ std::string Operator::resolveMatchMessage(Transaction *t, if (m_couldContainsMacro == false) { ret = "Matched \"Operator `" + m_op + "' with parameter `" + utils::string::limitTo(200, m_param) + - "' against variable `" + - utils::string::toHexIfNeeded(key, kEscapeLogMessage) + - "' (Value: `" + - (kEscapeLogMessage - ? utils::string::toHexIfNeeded( - utils::string::limitTo(100, value), true) - : utils::string::limitTo(100, - utils::string::toHexIfNeeded(value, false))) + \ + "' against variable `" + utils::string::toHexIfNeeded(key) + "' (Value: `" + + utils::string::limitTo(100, + utils::string::toHexIfNeeded(value)) + \ "' )"; } else { std::string p(m_string->evaluate(t)); ret = "Matched \"Operator `" + m_op + "' with parameter `" + - utils::string::toHexIfNeeded( - utils::string::limitTo(200, p), kEscapeLogMessage) + - "' against variable `" + - utils::string::toHexIfNeeded(key, kEscapeLogMessage) + - "' (Value: `" + - (kEscapeLogMessage - ? utils::string::toHexIfNeeded( - utils::string::limitTo(100, value), true) - : utils::string::limitTo(100, - utils::string::toHexIfNeeded(value, false))) + \ + utils::string::limitTo(200, p) + + "' against variable `" + utils::string::toHexIfNeeded(key) + "' (Value: `" + + utils::string::limitTo(100, + utils::string::toHexIfNeeded(value)) + "' )"; } } diff --git a/src/rule_message.cc b/src/rule_message.cc index b3155ffd0e..d1591c572d 100644 --- a/src/rule_message.cc +++ b/src/rule_message.cc @@ -98,6 +98,18 @@ std::string RuleMessage::_errorLogTail(const RuleMessage &rm) { } +/** + * Formats a rule match for textual logs without changing its stored contents. + * With MSC_LOG_MESSAGE_ESCAPE, the legacy match description is escaped as a + * whole, including backslashes in existing control-character representations. + * No limit is applied after escaping, so generated escapes remain complete. + * Bracketed fields are formatted separately to preserve their delimiters. + * + * @param rm Rule match to format; its contents remain available for JSON logs. + * @param props LogMessageInfo flags selecting optional client and tail fields. + * @param code Response status, or -1 to leave a status placeholder. + * @return Textual log entry with non-printable characters escaped. + */ std::string RuleMessage::log(const RuleMessage &rm, int props, int code) { std::string msg(""); msg.reserve(2048); @@ -119,7 +131,13 @@ std::string RuleMessage::log(const RuleMessage &rm, int props, int code) { msg.append("ModSecurity: Warning. "); } - msg.append(rm.m_match); + // Escape the stored match only for textual output. Its legacy contents + // must remain unchanged for JSON audit logs and connector callbacks. + if (kEscapeLogMessage) { + msg.append(utils::string::toHexIfNeeded(rm.m_match, true)); + } else { + msg.append(rm.m_match); + } msg.append(_details(rm)); if (props & ErrorLogTailLogMessageInfo) { diff --git a/test/test-cases/regression/issue-1528.json b/test/test-cases/regression/issue-1528.json index 53c8daa22b..aea06c160b 100644 --- a/test/test-cases/regression/issue-1528.json +++ b/test/test-cases/regression/issue-1528.json @@ -34,7 +34,7 @@ }, "expected": { "debug_log": "Rule returned 1", - "error_log": "Matched \"Operator `Rx' with parameter `\\^attack\\$'", + "error_log": "Operator `Rx' with parameter `\\^attack\\$'", "http_code": 200 }, "rules": [ diff --git a/test/test-cases/regression/issue-3601-legacy.json b/test/test-cases/regression/issue-3601-legacy.json index abe4f96fd8..b77906d6aa 100644 --- a/test/test-cases/regression/issue-3601-legacy.json +++ b/test/test-cases/regression/issue-3601-legacy.json @@ -130,5 +130,205 @@ "SecRuleEngine On", "SecRule REQUEST_METHOD \"@unconditionalMatch\" \"phase:1,id:10007,log,pass,msg:prefix%{REQUEST_METHOD}\\\"suffix\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with fixed parameter", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "alpha\"beta\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `Contains' with parameter `alpha' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\\"beta\\\\literal\\\\x22\\\\x0d\\\\x0aend' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \"Operator `Contains' with parameter `alpha' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\"beta\\literal\\x22\\x0d\\x0aend' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@contains alpha\" \"id:3601100,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-legacy-match.log" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with macro-expanded parameter", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "alpha\"beta\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `EndsWith' with parameter `alpha\\\"beta\\\\literal\\\\x22\\r\\nend' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\\"beta\\\\literal\\\\x22\\\\x0d\\\\x0aend' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \"Operator `EndsWith' with parameter `alpha\"beta\\literal\\x22\\x0d\\x0aend' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\"beta\\literal\\x22\\x0d\\x0aend' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@endsWith %{REQUEST_HEADERS:X-Log}\" \"id:3601101,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-legacy-match.log" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with value truncation boundary", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\\tail", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `Contains' with parameter `aaa' against variable `REQUEST_HEADERS:X-Log' (Value: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\"\\\\ (4 characters omitted)' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \"Operator `Contains' with parameter `aaa' against variable `REQUEST_HEADERS:X-Log' (Value: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\\ (4 characters omitted)' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@contains aaa\" \"id:3601102,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-legacy-match.log" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with fixed backslash parameter", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "alpha\"beta\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `Contains' with parameter `\\\\x22' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\\"beta\\\\literal\\\\x22\\\\x0d\\\\x0aend' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \"Operator `Contains' with parameter `\\x22' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\"beta\\literal\\x22\\x0d\\x0aend' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@contains \\x22\" \"id:3601103,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-legacy-match.log" + ] } ] diff --git a/test/test-cases/regression/issue-3601.json b/test/test-cases/regression/issue-3601.json index ebeeb4e46b..6b8c7043ed 100644 --- a/test/test-cases/regression/issue-3601.json +++ b/test/test-cases/regression/issue-3601.json @@ -79,7 +79,7 @@ ] }, "expected": { - "error_log": "(?=.*\\Q(Value: `/safe\\x22 ] [unique_id \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend' )\\E)(?=.*\\Q[msg \"request=message\\x22 ] [hostname \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"] [data \"\"] [severity \"0\"] [ver \"\"] [maturity \"0\"] [accuracy \"0\"] [hostname \"host.example\\x22 ] [uri \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"] [uri \"/safe\\x22 ] [unique_id \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"]\\E)", + "error_log": "(?=.*\\Q(Value: `/safe\\x22 ] [unique_id \\x22forged\\x5cliteral\\x5cx22\\x5cx0d\\x5cx0aend' )\\E)(?=.*\\Q[msg \"request=message\\x22 ] [hostname \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"] [data \"\"] [severity \"0\"] [ver \"\"] [maturity \"0\"] [accuracy \"0\"] [hostname \"host.example\\x22 ] [uri \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"] [uri \"/safe\\x22 ] [unique_id \\x22forged\\x5cliteral\\x5cx22\\x0d\\x0aend\"]\\E)", "http_code": 200 }, "rules": [ @@ -130,5 +130,205 @@ "SecRuleEngine On", "SecRule REQUEST_METHOD \"@unconditionalMatch\" \"phase:1,id:10007,log,pass,msg:prefix%{REQUEST_METHOD}\\\"suffix\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with fixed parameter", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "alpha\"beta\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `Contains' with parameter `alpha' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\\"beta\\\\literal\\\\x22\\\\x0d\\\\x0aend' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \\x22Operator `Contains' with parameter `alpha' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\x22beta\\x5cliteral\\x5cx22\\x5cx0d\\x5cx0aend' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@contains alpha\" \"id:3601100,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-match.log" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with macro-expanded parameter", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "alpha\"beta\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `EndsWith' with parameter `alpha\\\"beta\\\\literal\\\\x22\\r\\nend' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\\"beta\\\\literal\\\\x22\\\\x0d\\\\x0aend' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \\x22Operator `EndsWith' with parameter `alpha\\x22beta\\x5cliteral\\x5cx22\\x0d\\x0aend' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\x22beta\\x5cliteral\\x5cx22\\x5cx0d\\x5cx0aend' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@endsWith %{REQUEST_HEADERS:X-Log}\" \"id:3601101,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-match.log" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with value truncation boundary", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"\\tail", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `Contains' with parameter `aaa' against variable `REQUEST_HEADERS:X-Log' (Value: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\\"\\\\ (4 characters omitted)' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \\x22Operator `Contains' with parameter `aaa' against variable `REQUEST_HEADERS:X-Log' (Value: `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\\x22\\x5c (4 characters omitted)' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@contains aaa\" \"id:3601102,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-match.log" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Text and JSON match retains legacy output with fixed backslash parameter", + "github_issue": 3601, + "client": { + "ip": "192.0.2.10", + "port": 12345 + }, + "server": { + "ip": "192.0.2.20", + "port": 80, + "hostname": "normal.example" + }, + "request": { + "headers": { + "Host": "normal.example", + "X-Log": "alpha\"beta\\literal\\x22\r\nend", + "Content-Length": "0" + }, + "uri": "/normal", + "method": "GET", + "http_version": 1.1, + "body": [ + "" + ] + }, + "response": { + "headers": { + "Content-Length": "0" + }, + "body": [ + "" + ] + }, + "expected": { + "audit_log": "\\Q\"match\":\"Matched \\\"Operator `Contains' with parameter `\\\\x22' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\\"beta\\\\literal\\\\x22\\\\x0d\\\\x0aend' )\",\\E", + "http_code": 200, + "error_log": "\\QModSecurity: Warning. Matched \\x22Operator `Contains' with parameter `\\x5cx22' against variable `REQUEST_HEADERS:X-Log' (Value: `alpha\\x22beta\\x5cliteral\\x5cx22\\x5cx0d\\x5cx0aend' ) [file \"\\E" + }, + "rules": [ + "SecRuleEngine On", + "SecRule REQUEST_HEADERS:X-Log \"@contains \\x22\" \"id:3601103,phase:1,pass,log,auditlog,msg:'JSON match compatibility'\"", + "SecAuditEngine On", + "SecAuditLogFormat JSON", + "SecAuditLogType Serial", + "SecAuditLogParts ABHZ", + "SecAuditLog /tmp/issue-3601-match.log" + ] } ] diff --git a/test/test-cases/regression/operator-rx.json b/test/test-cases/regression/operator-rx.json index 0308819212..d33328fb7f 100644 --- a/test/test-cases/regression/operator-rx.json +++ b/test/test-cases/regression/operator-rx.json @@ -84,7 +84,7 @@ }, "expected": { "debug_log": "Executing operator \"Rx\" with param \"\\^0\\$\"", - "error_log": "Matched \"Operator `Rx' with parameter `\\^0\\$'", + "error_log": "Operator `Rx' with parameter `\\^0\\$'", "http_code": 200 }, "rules": [ @@ -177,7 +177,7 @@ }, "expected": { "debug_log": "rx: regex error 'MATCH_LIMIT' for pattern", - "error_log": "Matched \"Operator `StrEq' with parameter `1' against variable `MSC_PCRE_ERROR'", + "error_log": "Operator `StrEq' with parameter `1' against variable `MSC_PCRE_ERROR'", "http_code": 200 }, "rules": [ @@ -226,7 +226,7 @@ }, "expected": { "debug_log": "rx: regex error 'MATCH_LIMIT' for pattern", - "error_log": "Matched \"Operator `StrEq' with parameter `1' against variable `MSC_PCRE_LIMITS_EXCEEDED'", + "error_log": "Operator `StrEq' with parameter `1' against variable `MSC_PCRE_LIMITS_EXCEEDED'", "http_code": 200 }, "rules": [ @@ -275,7 +275,7 @@ }, "expected": { "debug_log": "rx: regex error 'MATCH_LIMIT' for pattern", - "error_log": "Matched \"Operator `StrEq' with parameter `1' against variable `TX:MSC_PCRE_LIMITS_EXCEEDED'", + "error_log": "Operator `StrEq' with parameter `1' against variable `TX:MSC_PCRE_LIMITS_EXCEEDED'", "http_code": 200 }, "rules": [