diff --git a/others/libinjection b/others/libinjection index b9fcaaf9e..211782219 160000 --- a/others/libinjection +++ b/others/libinjection @@ -1 +1 @@ -Subproject commit b9fcaaf9e50e9492807b23ffcc6af46ee1f203b9 +Subproject commit 211782219663f889f471650150df12b623c5766e diff --git a/src/operators/detect_sqli.cc b/src/operators/detect_sqli.cc index 49cef935c..05cefc543 100644 --- a/src/operators/detect_sqli.cc +++ b/src/operators/detect_sqli.cc @@ -17,45 +17,70 @@ #include #include +#include #include "src/operators/operator.h" +#include "src/operators/libinjection_utils.h" #include "libinjection/src/libinjection.h" +#include "libinjection/src/libinjection_error.h" -namespace modsecurity { -namespace operators { - +namespace modsecurity::operators { bool DetectSQLi::evaluate(Transaction *t, RuleWithActions *rule, const std::string& input, RuleMessage &ruleMessage) { - char fingerprint[8]; - int issqli; - issqli = libinjection_sqli(input.c_str(), input.length(), fingerprint); + std::array fingerprint{}; + + const injection_result_t sqli_result = + libinjection_sqli(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()); + + ms_dbg_a(t, 4, + std::string("detected SQLi using libinjection with fingerprint '") + + fingerprint.data() + "' at: '" + input + "'") + + 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: + ms_dbg_a(t, 4, + std::string("libinjection parser error during SQLi analysis (") + + libinjectionResultToString(sqli_result) + + "); treating as match (fail-safe). Input: '" + + input + "'") + + 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: ") + + input) + } + break; + + case LIBINJECTION_RESULT_FALSE: + ms_dbg_a(t, 9, + std::string("libinjection was not able to find any SQLi in: ") + + input) + break; } -tisempty: - return issqli != 0; + return isMaliciousLibinjectionResult(sqli_result); } - -} // namespace operators -} // namespace modsecurity +} // namespace modsecurity::operators diff --git a/src/operators/detect_xss.cc b/src/operators/detect_xss.cc index 014202e73..9782de537 100644 --- a/src/operators/detect_xss.cc +++ b/src/operators/detect_xss.cc @@ -18,36 +18,50 @@ #include #include "src/operators/operator.h" +#include "src/operators/libinjection_utils.h" #include "libinjection/src/libinjection.h" +#include "libinjection/src/libinjection_error.h" - -namespace modsecurity { -namespace operators { - +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)); + + const injection_result_t xss_result = + libinjection_xss(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: ") + input) } - } else { - ms_dbg_a(t, 9, "libinjection was not able to " \ - "find any XSS in: " + input); + break; + + case LIBINJECTION_RESULT_ERROR: + ms_dbg_a(t, 4, + std::string("libinjection parser error during XSS analysis (") + + libinjectionResultToString(xss_result) + + "); treating as match (fail-safe). Input: " + + input) + 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: ") + input) } + break; + + case LIBINJECTION_RESULT_FALSE: + ms_dbg_a(t, 9, + std::string("libinjection was not able to find any XSS in: ") + input) + break; } - return is_xss != 0; -} + return isMaliciousLibinjectionResult(xss_result); +} -} // namespace operators -} // namespace modsecurity +} // namespace modsecurity::operators diff --git a/src/operators/libinjection_utils.h b/src/operators/libinjection_utils.h new file mode 100644 index 000000000..dde3cc0f4 --- /dev/null +++ b/src/operators/libinjection_utils.h @@ -0,0 +1,48 @@ +/* + * 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. + * + */ + +#ifndef SRC_OPERATORS_LIBINJECTION_UTILS_H_ +#define SRC_OPERATORS_LIBINJECTION_UTILS_H_ + +#include "libinjection/src/libinjection_error.h" + +namespace modsecurity::operators { + +/* + * libinjection parser errors are handled in fail-safe mode as suspicious + * results, so callers can block on both confirmed detections and parser + * failures. + */ +static inline bool isMaliciousLibinjectionResult(injection_result_t result) { + return result == LIBINJECTION_RESULT_TRUE + || result == LIBINJECTION_RESULT_ERROR; +} + +static inline const char *libinjectionResultToString(injection_result_t result) { + switch (result) { + case LIBINJECTION_RESULT_TRUE: + return "attack-detected"; + case LIBINJECTION_RESULT_FALSE: + return "no-attack"; + case LIBINJECTION_RESULT_ERROR: + return "parser-error"; + } + + return "unexpected-result"; +} + +} // namespace modsecurity::operators + +#endif // SRC_OPERATORS_LIBINJECTION_UTILS_H_ \ No newline at end of file diff --git a/test/test-cases/regression/operator-detectsqli.json b/test/test-cases/regression/operator-detectsqli.json index 657bdf388..718f7a209 100644 --- a/test/test-cases/regression/operator-detectsqli.json +++ b/test/test-cases/regression/operator-detectsqli.json @@ -44,5 +44,187 @@ "SecRuleEngine On", "SecRule ARGS \"@detectSQLi\" \"id:1,phase:2,capture,pass,t:trim\"" ] + },[ + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Operator :: @detectSQLi :: known fingerprint payload", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "61", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=ascii(substring(version() from 1 for 1))¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Added DetectSQLi match TX.0: f\\(f\\(f", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule ARGS \"@detectSQLi\" \"id:1,phase:2,capture,pass,t:trim\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Operator :: @detectSQLi :: trim still captures fingerprint", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "67", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1= ascii(substring(version() from 1 for 1)) ¶m2=value2" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Added DetectSQLi match TX.0: f\\(f\\(f", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule ARGS \"@detectSQLi\" \"id:2,phase:2,capture,pass,t:trim\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Operator :: @detectSQLi :: boolean style payload", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "31", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=' or 1=1 -- ¶m2=x" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule ARGS \"@detectSQLi\" \"id:3,phase:2,capture,pass,t:trim\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Operator :: @detectSQLi :: benign input should not match", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "24", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=hello-world&x=1" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule ARGS \"@detectSQLi\" \"id:4,phase:2,capture,pass,t:trim\"" + ] } ] diff --git a/test/test-cases/regression/operator-detectxss.json b/test/test-cases/regression/operator-detectxss.json index 9e6fa24b2..1fa751e1c 100644 --- a/test/test-cases/regression/operator-detectxss.json +++ b/test/test-cases/regression/operator-detectxss.json @@ -44,5 +44,188 @@ "SecRuleEngine On", "SecRule ARGS \"@detectXSS\" \"id:1,phase:2,capture,pass,t:trim\"" ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Operator :: @detectXSS :: basic script payload", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "45", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=alert(1)&p=1" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "debug_log": "Added DetectXSS match TX.0: ", + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule ARGS \"@detectXSS\" \"id:3,phase:2,capture,pass,t:trim\"" + ] + }, + { + "enabled": 1, + "version_min": 300000, + "title": "Testing Operator :: @detectXSS :: benign input should not match", + "client": { + "ip": "200.249.12.31", + "port": 123 + }, + "server": { + "ip": "200.249.12.31", + "port": 80 + }, + "request": { + "headers": { + "Host": "localhost", + "User-Agent": "curl/7.38.0", + "Accept": "*/*", + "Content-Length": "24", + "Content-Type": "application/x-www-form-urlencoded" + }, + "uri": "/", + "method": "POST", + "body": [ + "param1=hello-world&x=1" + ] + }, + "response": { + "headers": { + "Date": "Mon, 13 Jul 2015 20:02:41 GMT", + "Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT", + "Content-Type": "text/html", + "Content-Length": "8" + }, + "body": [ + "no need." + ] + }, + "expected": { + "http_code": 200 + }, + "rules": [ + "SecRuleEngine On", + "SecRule ARGS \"@detectXSS\" \"id:4,phase:2,capture,pass,t:trim\"" + ] } ]