diff --git a/src/assay.cc b/src/assay.cc index ee603123..40e56f25 100644 --- a/src/assay.cc +++ b/src/assay.cc @@ -426,16 +426,52 @@ int Assay::processRequestBody() { std::string *a = resolve_variable_first("REQUEST_HEADERS:Content-Type"); if (a != NULL) { Multipart m(*a); - m.init(); - m.process(m_requestBody.str()); - for (auto &a : m.variables) { - store_variable(a.first, a.second); - } - if (m.crlf && m.lf) { - store_variable("MULTIPART_CRLF_LF_LINES", "1"); - } else { - store_variable("MULTIPART_CRLF_LF_LINES", "0"); + if (m.init() == true) { + m.process(m_requestBody.str()); + for (auto &a : m.variables) { + store_variable(a.first, a.second); + } + if (m.crlf && m.lf) { + store_variable("MULTIPART_CRLF_LF_LINES", "1"); + } else { + store_variable("MULTIPART_CRLF_LF_LINES", "0"); + } + if (m.boundaryStartsWithWhiteSpace) { + debug(9, "Multipart: Boundary starts with white space, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } + if (m.boundaryIsQuoted) { + debug(9, "Multipart: Boundary is quoted, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } + if (m.containsDataAfter) { + debug(9, "Multipart: There is data after the boundary, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } + if (m.containsDataBefore) { + debug(9, "Multipart: There is data before the boundary, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } + if (m.lf) { + debug(9, "Multipart: Lines are LF-terminated, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } + if (m.missingSemicolon) { + debug(9, "Multipart: Boundary missing semicolon, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } + if (m.invalidQuote) { + debug(9, "Multipart: Invalid quote, " \ + "setting MULTIPART_STRICT_ERROR to 1"); + update_variable_first("MULTIPART_STRICT_ERROR", "1"); + } } } } diff --git a/src/parser/seclang-scanner.ll b/src/parser/seclang-scanner.ll index 899424fc..e686e381 100755 --- a/src/parser/seclang-scanner.ll +++ b/src/parser/seclang-scanner.ll @@ -59,7 +59,7 @@ OPERATORNOARG (?i:@detectSQLi|@detectXSS|@geoLookup|@validateUrlEncoding|@valida TRANSFORMATION t:(lowercase|urlDecodeUni|urlDecode|none|compressWhitespace|removeWhitespace|replaceNulls|removeNulls|htmlEntityDecode|jsDecode|cssDecode|trim) -VARIABLE (?i:MULTIPART_NAME|MULTIPART_FILENAME|MULTIPART_CRLF_LF_LINES|MATCHED_VAR_NAME|MATCHED_VARS_NAMES|MATCHED_VAR|MATCHED_VARS|INBOUND_DATA_ERROR|FULL_REQUEST|FILES|AUTH_TYPE|ARGS_NAMES|ARGS|QUERY_STRING|REMOTE_ADDR|REQUEST_BASENAME|REQUEST_BODY|REQUEST_COOKIES_NAMES|REQUEST_COOKIES|REQUEST_FILENAME|REQUEST_HEADERS_NAMES|REQUEST_HEADERS|REQUEST_METHOD|REQUEST_PROTOCOL|REQUEST_URI|RESPONSE_BODY|RESPONSE_CONTENT_LENGTH|RESPONSE_CONTENT_TYPE|RESPONSE_HEADERS_NAMES|RESPONSE_HEADERS|RESPONSE_PROTOCOL|RESPONSE_STATUS|TX|GEO) +VARIABLE (?i:MULTIPART_STRICT_ERROR|MULTIPART_NAME|MULTIPART_FILENAME|MULTIPART_CRLF_LF_LINES|MATCHED_VAR_NAME|MATCHED_VARS_NAMES|MATCHED_VAR|MATCHED_VARS|INBOUND_DATA_ERROR|FULL_REQUEST|FILES|AUTH_TYPE|ARGS_NAMES|ARGS|QUERY_STRING|REMOTE_ADDR|REQUEST_BASENAME|REQUEST_BODY|REQUEST_COOKIES_NAMES|REQUEST_COOKIES|REQUEST_FILENAME|REQUEST_HEADERS_NAMES|REQUEST_HEADERS|REQUEST_METHOD|REQUEST_PROTOCOL|REQUEST_URI|RESPONSE_BODY|RESPONSE_CONTENT_LENGTH|RESPONSE_CONTENT_TYPE|RESPONSE_HEADERS_NAMES|RESPONSE_HEADERS|RESPONSE_PROTOCOL|RESPONSE_STATUS|TX|GEO) RUN_TIME_VAR_DUR (?i:DURATION) RUN_TIME_VAR_ENV (?i:ENV) RUN_TIME_VAR_BLD (?i:MODSEC_BUILD) diff --git a/src/request_body_processor/multipart.cc b/src/request_body_processor/multipart.cc index 4e2352e8..da6c7d85 100644 --- a/src/request_body_processor/multipart.cc +++ b/src/request_body_processor/multipart.cc @@ -26,9 +26,12 @@ namespace RequestBodyProcessor { Multipart::Multipart(std:: string header) : crlf(false), - lf(true), - m_boundaryStartsWithWhiteSpace(false), - m_boundaryIsQuoted(false), + containsDataAfter(false), + containsDataBefore(false), + lf(false), + missingSemicolon(false), + boundaryStartsWithWhiteSpace(false), + boundaryIsQuoted(false), m_header(header) { } @@ -57,8 +60,9 @@ bool Multipart::init() { return false; } - if (semicolon_pos < 0) { + if (semicolon_pos == std::string::npos) { debug(4, "Multipart: Missing semicolon."); + this->missingSemicolon = true; } if (boundary.at(8) != '=') { @@ -66,46 +70,19 @@ bool Multipart::init() { "Missing equals."); return false; } -#if 0 - Not checked. - /* Check parameter name ends well. */ - if (b != (msr->mpd->boundary + 8)) { - /* Check all characters between the end of the boundary - * and the = character. - */ - for (p = msr->mpd->boundary + 8; p < b; p++) { - if (isspace(*p)) { - /* Flag for whitespace after parameter name. */ - msr->mpd->flag_boundary_whitespace = 1; - } else { - msr->mpd->flag_error = 1; - *error_msg = apr_psprintf(msr->mp, "Multipart: " \ - "Invalid boundary in C-T (parameter name)."); - return -1; - } - } - } - b++; /* Go over the = character. */ - len = strlen(b); - - /* Flag for whitespace before parameter value. */ - if (isspace(*b)) { - msr->mpd->flag_boundary_whitespace = 1; - } -#endif // if 0 if (boundary.at(8 + 1) == ' ') { - m_boundaryStartsWithWhiteSpace = true; + boundaryStartsWithWhiteSpace = true; debug(4, "Multipart: Boundary starts with a white space"); } - if ((m_boundaryStartsWithWhiteSpace && boundary.at(8 + 2) == '"') || - (!m_boundaryStartsWithWhiteSpace && boundary.at(8 + 1) == '"')) { - m_boundaryIsQuoted = true; + if ((boundaryStartsWithWhiteSpace && boundary.at(8 + 2) == '"') || + (!boundaryStartsWithWhiteSpace && boundary.at(8 + 1) == '"')) { + boundaryIsQuoted = true; debug(4, "Multipart: Boundary inside quotes"); } - if (m_boundaryIsQuoted && boundary.at(boundary.length()-1) != '"') { + if (boundaryIsQuoted && boundary.at(boundary.length()-1) != '"') { debug(4, "Multipart: Invalid boundary in Content-type (quote)."); return false; } @@ -123,17 +100,17 @@ bool Multipart::init() { int real_boundary_pos = 9; - if (m_boundaryStartsWithWhiteSpace) { + if (boundaryStartsWithWhiteSpace) { real_boundary_pos++; } - if (m_boundaryIsQuoted) { + if (boundaryIsQuoted) { real_boundary_pos++; } m_boundary = boundary.c_str() + real_boundary_pos; - if (m_boundaryIsQuoted) { + if (boundaryIsQuoted) { m_boundary.pop_back(); } @@ -205,9 +182,13 @@ bool Multipart::process(std::string data) { std::list blobs; size_t start = data.find(m_boundary); size_t endl = 1; + size_t lastValidBoundary = 0; + size_t firstValidBoundary = start; + double files_size = 0; if (start != 0) { debug(4, "Multipart: Boundary was not the first thing."); + this->containsDataBefore = true; } while (start != std::string::npos) { @@ -222,10 +203,22 @@ bool Multipart::process(std::string data) { checkForCrlfLf(block); blobs.push_back(block); + lastValidBoundary = end; start = end; } - double files_size = 0; + size_t lastPiece = m_boundary.length() + lastValidBoundary \ + + firstValidBoundary + 2; + if (this->crlf) { + lastPiece = lastPiece + 2; + } else { + lastPiece = lastPiece + 1; + } + + if (data.length() > lastPiece) { + this->containsDataAfter = true; + } + std::string filename(""); std::string name(""); for (std::string x : blobs) { @@ -243,6 +236,10 @@ bool Multipart::process(std::string data) { variables.emplace("FILES_TMP_CONTENT:" + m.name, m.content); files_size = files_size + m.content.size(); } + if (m.invalidQuote) { + debug(4, "Multipart: Found invalid quoting."); + this->invalidQuote = true; + } } if (filename.empty() == false) { variables.emplace("MULTIPART_FILENAME", filename); diff --git a/src/request_body_processor/multipart.h b/src/request_body_processor/multipart.h index 496a6f14..0150ff05 100644 --- a/src/request_body_processor/multipart.h +++ b/src/request_body_processor/multipart.h @@ -36,17 +36,22 @@ class Multipart { void checkForCrlfLf(const std::string &blob); ModSecurityStringVariables variables; + bool crlf; + bool containsDataAfter; + bool containsDataBefore; bool lf; + bool boundaryStartsWithWhiteSpace; + bool boundaryIsQuoted; + bool missingSemicolon; + bool invalidQuote; + private: void debug(int a, std::string str) { std::cout << "Debug: " << str << std::endl; } std::string m_boundary; std::string m_header; - - bool m_boundaryStartsWithWhiteSpace = false; - bool m_boundaryIsQuoted = false; }; } // namespace RequestBodyProcessor diff --git a/src/request_body_processor/multipart_blob.cc b/src/request_body_processor/multipart_blob.cc index 1d6e8213..9e2ffeb3 100644 --- a/src/request_body_processor/multipart_blob.cc +++ b/src/request_body_processor/multipart_blob.cc @@ -25,6 +25,7 @@ namespace RequestBodyProcessor { MultipartBlob::MultipartBlob(const std::string &blob, Multipart *parent) : m_blob(blob), + invalidQuote(false), m_parent(parent) { processContent(); } @@ -98,6 +99,10 @@ bool MultipartBlob::processContentDispositionLine( // Find name= offset = dispositionLine.find("name="); if (offset != std::string::npos) { + size_t invalidQuote = dispositionLine.find("\'", offset); + if (invalidQuote != std::string::npos) { + this->invalidQuote = true; + } offset = offset + 5 /* name= */ + 1 /* " */; size_t end = dispositionLine.find("\"", offset); if (end != std::string::npos) { @@ -108,6 +113,10 @@ bool MultipartBlob::processContentDispositionLine( // Find filename= offset = dispositionLine.find("filename="); if (offset != std::string::npos) { + size_t invalidQuote = dispositionLine.find("\'", offset); + if (invalidQuote != std::string::npos) { + this->invalidQuote = true; + } offset = offset + 9 /* filename= */ + 1 /* " */; size_t end = dispositionLine.find("\"", offset); if (end != std::string::npos) { diff --git a/src/request_body_processor/multipart_blob.h b/src/request_body_processor/multipart_blob.h index c4a9e0a1..9cf3ab56 100644 --- a/src/request_body_processor/multipart_blob.h +++ b/src/request_body_processor/multipart_blob.h @@ -36,6 +36,7 @@ class MultipartBlob { std::cout << "Debug: " << str << std::endl; } + bool invalidQuote; std::string name; std::string filename; std::string contentType; diff --git a/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json b/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json new file mode 100644 index 00000000..f434106c --- /dev/null +++ b/test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json @@ -0,0 +1,369 @@ +[ + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR", + "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":"330", + "Content-Type":"multipart/form-data; boundary= --------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "protocol":"POST", + "body":[ + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "--------------------------756b6d74fa1a8ee2--" + ] + }, + "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" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Multipart: Boundary starts with white space, setting MULTIPART_STRICT_ERROR to 1" + }, + "rules":[ + "SecRuleEngine On", + "SecDebugLog \/tmp\/modsec_debug.log", + "SecDebugLogLevel 9", + "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"phase:3,pass,t:trim\"" + ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR", + "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":"330", + "Content-Type":"multipart/form-data; boundary=\"--------------------------756b6d74fa1a8ee2\"", + "Expect":"100-continue" + }, + "uri":"/", + "protocol":"POST", + "body":[ + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "--------------------------756b6d74fa1a8ee2--" + ] + }, + "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" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Multipart: Boundary is quoted, setting MULTIPART_STRICT_ERROR to 1" + }, + "rules":[ + "SecRuleEngine On", + "SecDebugLog \/tmp\/modsec_debug.log", + "SecDebugLogLevel 9", + "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"phase:3,pass,t:trim\"" + ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR", + "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":"330", + "Content-Type":"multipart/form-data; boundary=--------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "protocol":"POST", + "body":[ + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "--------------------------756b6d74fa1a8ee2--whee." + ] + }, + "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" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Multipart: There is data after the boundary, setting MULTIPART_STRICT_ERROR to 1" + }, + "rules":[ + "SecRuleEngine On", + "SecDebugLog \/tmp\/modsec_debug.log", + "SecDebugLogLevel 9", + "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"phase:3,pass,t:trim\"" + ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR", + "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":"330", + "Content-Type":"multipart/form-data; boundary=--------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "protocol":"POST", + "body":[ + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "--------------------------756b6d74fa1a8ee2--" + ] + }, + "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" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Multipart: Lines are LF-terminated, setting MULTIPART_STRICT_ERROR to 1" + }, + "rules":[ + "SecRuleEngine On", + "SecDebugLog \/tmp\/modsec_debug.log", + "SecDebugLogLevel 9", + "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"phase:3,pass,t:trim\"" + ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR", + "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":"330", + "Content-Type":"multipart/form-data; boundary=--------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "protocol":"POST", + "body":[ + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "--------------------------756b6d74fa1a8ee2--" + ] + }, + "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" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Boundary missing semicolon, setting MULTIPART_STRICT_ERROR to 1" + }, + "rules":[ + "SecRuleEngine On", + "SecDebugLog \/tmp\/modsec_debug.log", + "SecDebugLogLevel 9", + "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"phase:3,pass,t:trim\"" + ] + }, + { + "enabled":1, + "version_min":300000, + "title":"Testing Variables :: MULTIPART_STRICT_ERROR", + "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":"330", + "Content-Type":"multipart/form-data; boundary=--------------------------756b6d74fa1a8ee2", + "Expect":"100-continue" + }, + "uri":"/", + "protocol":"POST", + "body":[ + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"name\"", + "", + "test", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name='filedata'; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is a very small test file..", + "--------------------------756b6d74fa1a8ee2", + "Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"", + "Content-Type: text/plain", + "", + "This is another very small test file..", + "--------------------------756b6d74fa1a8ee2--" + ] + }, + "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" + }, + "body":[ + "no need." + ] + }, + "expected":{ + "debug_log":"Multipart: Invalid quote, setting MULTIPART_STRICT_ERROR to 1" + }, + "rules":[ + "SecRuleEngine On", + "SecDebugLog \/tmp\/modsec_debug.log", + "SecDebugLogLevel 9", + "SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"phase:3,pass,t:trim\"" + ] + } +] +