Update SQLi/XSS operators for libinjection v4.0.0 cleaned

build/win32/CMakeLists.txt

@@ -164,7 +164,7 @@ project(libModSecurityTests)
 
 function(setTestTargetProperties executable)
   target_compile_definitions(${executable} PRIVATE WITH_PCRE2)
-  target_include_directories(${executable} PRIVATE ${BASE_DIR} ${BASE_DIR}/headers)
+  target_include_directories(${executable} PRIVATE ${BASE_DIR} ${BASE_DIR}/headers ${BASE_DIR}/others)
   target_link_libraries(${executable} PRIVATE libModSecurity pcre2::pcre2 dirent::dirent)
   add_package_dependency(${executable} WITH_YAJL yajl::yajl HAVE_YAJL)
 endfunction()
@@ -239,7 +239,7 @@ setTestTargetProperties(rules_optimization)
 project(libModSecurityExamples)
 
 function(setExampleTargetProperties executable)
-  target_include_directories(${executable} PRIVATE ${BASE_DIR} ${BASE_DIR}/headers)
+  target_include_directories(${executable} PRIVATE ${BASE_DIR} ${BASE_DIR}/headers ${BASE_DIR}/others)
   target_link_libraries(${executable} PRIVATE libModSecurity)
 endfunction()
 

others/Makefile.am

@@ -15,6 +15,7 @@ noinst_HEADERS = \
 	libinjection/src/libinjection_sqli.h \
 	libinjection/src/libinjection_sqli_data.h \
 	libinjection/src/libinjection_xss.h \
+	libinjection/src/libinjection_error.h \
 	mbedtls/include/mbedtls/base64.h \
 	mbedtls/include/mbedtls/check_config.h \
 	mbedtls/include/mbedtls/mbedtls_config.h \

src/Makefile.am

@@ -187,6 +187,7 @@ OPERATORS = \
 	operators/contains_word.cc \
 	operators/detect_sqli.cc \
 	operators/detect_xss.cc \
+	operators/libinjection_adapter.cc \
 	operators/ends_with.cc \
 	operators/eq.cc \
 	operators/fuzzy_hash.cc \

src/actions/init_col.cc

@@ -21,6 +21,7 @@
 #include "modsecurity/actions/action.h"
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule.h"
+#include "src/utils/string.h"
 
 
 namespace modsecurity {
@@ -68,7 +69,7 @@
     }
 
     ms_dbg_a(t, 5, "Collection `" + m_collection_key + "' initialized with " \
-        "value: " + collectionName);
+        "value: " + utils::string::safeLogValue(collectionName));
 
     return true;
 }

src/actions/msg.cc

@@ -23,6 +23,7 @@
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule.h"
 #include "modsecurity/rule_message.h"
+#include "src/utils/string.h"
 
 /*
  * Description: Assigns a custom message to the rule or chain in which it
@@ -49,7 +50,7 @@
 bool Msg::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
     const auto msg = data(transaction);
     ruleMessage.m_message = msg;
-    ms_dbg_a(transaction, 9, "Saving msg: " + msg);
+    ms_dbg_a(transaction, 9, "Saving msg: " + utils::string::safeLogValue(msg));
 
     return true;
 }

src/actions/set_env.cc

@@ -31,7 +31,7 @@ bool SetENV::evaluate(RuleWithActions *rule, Transaction *t) {
 
     auto pair = utils::string::ssplit_pair(colNameExpanded, '=');
     ms_dbg_a(t, 8, "Setting environment variable: "
-        + pair.first + " to " + pair.second);
+        + pair.first + " to " + utils::string::safeLogValue(pair.second));
 #ifndef WIN32
     setenv(pair.first.c_str(), pair.second.c_str(), /*overwrite*/ 1);
 #else

src/actions/set_sid.cc

@@ -20,6 +20,7 @@
 
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule.h"
+#include "src/utils/string.h"
 
 
 namespace modsecurity {
@@ -29,7 +30,7 @@
 bool SetSID::evaluate(RuleWithActions *rule, Transaction *t) {
     std::string colNameExpanded(m_string->evaluate(t));
     ms_dbg_a(t, 8, "Session ID initiated with value: \'"
-        + colNameExpanded + "\'.");
+        + utils::string::safeLogValue(colNameExpanded) + "\'.");
 
     t->m_collections.m_session_collection_key = colNameExpanded;
     t->m_variableSessionID.set(colNameExpanded, t->m_variableOffset);

src/actions/set_uid.cc

@@ -20,6 +20,7 @@
 
 #include "modsecurity/transaction.h"
 #include "modsecurity/rule.h"
+#include "src/utils/string.h"
 
 
 namespace modsecurity {
@@ -29,7 +30,7 @@
 bool SetUID::evaluate(RuleWithActions *rule, Transaction *t) {
     std::string colNameExpanded(m_string->evaluate(t));
     ms_dbg_a(t, 8, "User collection initiated with value: \'"
-        + colNameExpanded + "\'.");
+        + utils::string::safeLogValue(colNameExpanded) + "\'.");
 
     t->m_collections.m_user_collection_key = colNameExpanded;
     t->m_variableUserID.set(colNameExpanded, t->m_variableOffset);

src/actions/set_var.cc

@@ -123,7 +123,8 @@
     }
 
     ms_dbg_a(t, 8, "Saving variable: " + m_variable->m_collectionName \
-        + ":" + m_variableNameExpanded + " with value: " + targetValue);
+        + ":" + m_variableNameExpanded + " with value: " \
+        + utils::string::safeLogValue(targetValue));
 
     if (tx) {
         tx->storeOrUpdateFirst(t, m_variableNameExpanded, targetValue);

src/engine/lua.cc

@@ -185,7 +185,7 @@ int Lua::run(Transaction *t, const std::string &str) { // cppcheck-suppress cons
 
     lua_getglobal(L, "main");
 
-    ms_dbg_a(t, 9, str);
+    ms_dbg_a(t, 9, utils::string::safeLogMetadata("Lua argument", str));
 
     /* Put the parameter on the stack. */
     if (!str.empty() ) {
@@ -211,7 +211,8 @@ int Lua::run(Transaction *t, const std::string &str) { // cppcheck-suppress cons
         luaRet.assign(a);
     }
 
-    ms_dbg_a(t, 9, "Returning from lua script: " + luaRet);
+    ms_dbg_a(t, 9, "Returning from lua script: "
+        + utils::string::safeLogValue(luaRet));
 
     if (luaRet.size() == 0) {
         ret = false;
@@ -474,4 +475,3 @@ void Lua::applyTransformations(lua_State *L, const Transaction *t,
 
 }  //  namespace engine
 }  //  namespace modsecurity
-

src/operators/detect_sqli.cc

@@ -17,45 +17,101 @@
 
 #include <string>
 #include <list>
+#include <array>
 
 #include "src/operators/operator.h"
-#include "libinjection/src/libinjection.h"
-
-namespace modsecurity {
-namespace operators {
+#include "src/operators/libinjection_utils.h"
+#include "src/operators/libinjection_adapter.h"
+#include "src/utils/string.h"
+#include "libinjection/src/libinjection_error.h"
 
+namespace modsecurity::operators {
 
 bool DetectSQLi::evaluate(Transaction *t, RuleWithActions *rule,
     const std::string& input, RuleMessage &ruleMessage) {
-    char fingerprint[8];
-    int issqli;
+#ifndef NO_LOGS
+    const std::string loggable_input = utils::string::safeLogValue(input);
+#endif
+
+    std::array<char, 8> fingerprint{};
 
-    issqli = libinjection_sqli(input.c_str(), input.length(), fingerprint);
+    const injection_result_t sqli_result =
+        runLibinjectionSQLi(input.c_str(), input.length(), fingerprint.data());
 
-    if (!t) {
-        goto tisempty;
+    if (t == nullptr) {
+        return isMaliciousLibinjectionResult(sqli_result);
     }
 
-    if (issqli) {
-        t->m_matched.push_back(fingerprint);
-        ms_dbg_a(t, 4, "detected SQLi using libinjection with " \
-            "fingerprint '" + std::string(fingerprint) + "' at: '" +
-            input + "'");
-        if (rule && rule->hasCaptureAction()) {
-            t->m_collections.m_tx_collection->storeOrUpdateFirst(
-                "0", std::string(fingerprint));
-            ms_dbg_a(t, 7, "Added DetectSQLi match TX.0: " + \
-                std::string(fingerprint));
-        }
-    } else {
-        ms_dbg_a(t, 9, "detected SQLi: not able to find an " \
-            "inject on '" + input + "'");
+    switch (sqli_result) {
+        case LIBINJECTION_RESULT_TRUE:
+            t->m_matched.emplace_back(fingerprint.data());
+
+#ifndef NO_LOGS
+            ms_dbg_a(t, 4,
+                std::string("detected SQLi using libinjection with fingerprint '")
+                + fingerprint.data() + "' at: '" + loggable_input + "'");
+#endif
+
+            if (rule != nullptr && rule->hasCaptureAction()) {
+                t->m_collections.m_tx_collection->storeOrUpdateFirst(
+                    "0", std::string(fingerprint.data()));
+
+                ms_dbg_a(t, 7,
+                    std::string("Added DetectSQLi match TX.0: ")
+                    + fingerprint.data());
+            }
+            break;
+
+        case LIBINJECTION_RESULT_ERROR:
+#ifndef NO_LOGS
+            ms_dbg_a(t, 4,
+                std::string("libinjection parser error during SQLi analysis (")
+                + libinjectionResultToString(sqli_result)
+                + "); treating as match (fail-safe). Input: '"
+                + loggable_input + "'");
+#endif
+
+            if (rule != nullptr && rule->hasCaptureAction()) {
+                t->m_collections.m_tx_collection->storeOrUpdateFirst(
+                    "0", input);
+
+                ms_dbg_a(t, 7,
+                    std::string("Added DetectSQLi error input TX.0: ")
+                    + loggable_input);
+            }
+
+            // Keep m_matched untouched for parser-error paths to avoid
+            // introducing synthetic fingerprints for non-TRUE results.
+            break;
+
+        case LIBINJECTION_RESULT_FALSE:
+#ifndef NO_LOGS
+            ms_dbg_a(t, 9,
+                std::string("libinjection was not able to find any SQLi in: ")
+                + loggable_input);
+#endif
+            break;
+
+        default:
+#ifndef NO_LOGS
+            ms_dbg_a(t, 4,
+                std::string("libinjection returned unexpected SQLi result (")
+                + libinjectionResultToString(sqli_result)
+                + "); treating as match (fail-safe). Input: '"
+                + loggable_input + "'");
+#endif
+            if (rule != nullptr && rule->hasCaptureAction()) {
+                t->m_collections.m_tx_collection->storeOrUpdateFirst(
+                    "0", input);
+
+                ms_dbg_a(t, 7,
+                    std::string("Added DetectSQLi unexpected input TX.0: ")
+                    + loggable_input);
+            }
+            break;
     }
 
-tisempty:
-    return issqli != 0;
+    return isMaliciousLibinjectionResult(sqli_result);
 }
 
-
-}  // namespace operators
-}  // namespace modsecurity
+}  // namespace modsecurity::operators

src/operators/detect_xss.cc

@@ -18,36 +18,74 @@
 #include <string>
 
 #include "src/operators/operator.h"
-#include "libinjection/src/libinjection.h"
-
-
-namespace modsecurity {
-namespace operators {
+#include "src/operators/libinjection_utils.h"
+#include "src/operators/libinjection_adapter.h"
+#include "src/utils/string.h"
+#include "libinjection/src/libinjection_error.h"
 
+namespace modsecurity::operators {
 
 bool DetectXSS::evaluate(Transaction *t, RuleWithActions *rule,
     const std::string& input, RuleMessage &ruleMessage) {
-    int is_xss;
-
-    is_xss = libinjection_xss(input.c_str(), input.length());
-
-    if (t) {
-        if (is_xss) {
-            ms_dbg_a(t, 5, "detected XSS using libinjection.");
-            if (rule && rule->hasCaptureAction()) {
-                t->m_collections.m_tx_collection->storeOrUpdateFirst(
-                    "0", std::string(input));
-                ms_dbg_a(t, 7, "Added DetectXSS match TX.0: " + \
-                    std::string(input));
+#ifndef NO_LOGS
+    const std::string loggable_input = utils::string::safeLogValue(input);
+#endif
+
+    const injection_result_t xss_result =
+        runLibinjectionXSS(input.c_str(), input.length());
+
+    if (t == nullptr) {
+        return isMaliciousLibinjectionResult(xss_result);
+    }
+
+    switch (xss_result) {
+        case LIBINJECTION_RESULT_TRUE:
+            ms_dbg_a(t, 5, std::string("detected XSS using libinjection."));
+            if (rule != nullptr && rule->hasCaptureAction()) {
+                t->m_collections.m_tx_collection->storeOrUpdateFirst("0", input);
+                ms_dbg_a(t, 7, std::string("Added DetectXSS match TX.0: ") + loggable_input);
             }
-        } else {
-            ms_dbg_a(t, 9, "libinjection was not able to " \
-                "find any XSS in: " + input);
+            break;
+
+        case LIBINJECTION_RESULT_ERROR:
+#ifndef NO_LOGS
+            ms_dbg_a(t, 4,
+                std::string("libinjection parser error during XSS analysis (")
+                + libinjectionResultToString(xss_result)
+                + "); treating as match (fail-safe). Input: "
+                + loggable_input);
+#endif
+            if (rule != nullptr && rule->hasCaptureAction()) {
+                t->m_collections.m_tx_collection->storeOrUpdateFirst("0", input);
+                ms_dbg_a(t, 7, std::string("Added DetectXSS error input TX.0: ") + loggable_input);
+            }
+            break;
+
+        case LIBINJECTION_RESULT_FALSE:
+#ifndef NO_LOGS
+            ms_dbg_a(t, 9,
+                std::string("libinjection was not able to find any XSS in: ")
+                + loggable_input);
+#endif
+            break;
+
+        default:
+#ifndef NO_LOGS
+            ms_dbg_a(t, 4,
+                std::string("libinjection returned unexpected XSS result (")
+                + libinjectionResultToString(xss_result)
+                + "); treating as match (fail-safe). Input: "
+                + loggable_input);
+#endif
+            if (rule != nullptr && rule->hasCaptureAction()) {
+                t->m_collections.m_tx_collection->storeOrUpdateFirst("0", input);
+                ms_dbg_a(t, 7,
+                    std::string("Added DetectXSS unexpected input TX.0: ") + loggable_input);
             }
+            break;
     }
-    return is_xss != 0;
-}
 
+    return isMaliciousLibinjectionResult(xss_result);
+}
 
-}  // namespace operators
-}  // namespace modsecurity
+}  // namespace modsecurity::operators