github_mirrors/ModSecurity
3
0
Fork 1
mirror of https://github.com/owasp-modsecurity/ModSecurity.git synced 2025-08-13 13:26:01 +03:00
Code Issues Packages Projects Releases Wiki Activity

Multipart parsing fixes and new MULTIPART_PART_HEADERS collection

Browse Source
This commit is contained in:
Martin Vierula 2022-09-07 06:29:20 -07:00
parent 648cad380e
commit fa6e41857d
No known key found for this signature in database
GPG Key ID: F2FC4E45883BCBA4
13 changed files with 6569 additions and 6151 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CHANGES
View File

@ -1,6 +1,8 @@
v3.x.y - YYYY-MMM-DD (to be released)
-------------------------------------
- Multipart parsing fixes and new MULTIPART_PART_HEADERS collection
[Issue #2795 - @terjanq, @martinhsv]
- Prevent LMDB related segfault
[Issue #2755, #2761 - @dvershinin]
- Fix msc_transaction_cleanup function comment typo

1
Makefile.am
View File

@ -225,6 +225,7 @@ TESTS+=test/test-cases/regression/variable-MULTIPART_CRLF_LF_LINES.json
TESTS+=test/test-cases/regression/variable-MULTIPART_FILENAME.json
TESTS+=test/test-cases/regression/variable-MULTIPART_INVALID_HEADER_FOLDING.json
TESTS+=test/test-cases/regression/variable-MULTIPART_NAME.json
TESTS+=test/test-cases/regression/variable-MULTIPART_PART_HEADERS.json
TESTS+=test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json
TESTS+=test/test-cases/regression/variable-MULTIPART_UNMATCHED_BOUNDARY.json
TESTS+=test/test-cases/regression/variable-OUTBOUND_DATA_ERROR.json

2
headers/modsecurity/transaction.h
View File

@ -201,6 +201,7 @@ class TransactionAnchoredVariables {
m_variableGeo(t, "GEO"),
m_variableRequestCookiesNames(t, "REQUEST_COOKIES_NAMES"),
m_variableFilesTmpNames(t, "FILES_TMPNAMES"),
m_variableMultipartPartHeaders(t, "MULTIPART_PART_HEADERS"),
m_variableOffset(0),
m_variableArgsNames("ARGS_NAMES", &m_variableArgs),
m_variableArgsGetNames("ARGS_GET_NAMES", &m_variableArgsGet),
@ -282,6 +283,7 @@ class TransactionAnchoredVariables {
AnchoredSetVariable m_variableGeo;
AnchoredSetVariable m_variableRequestCookiesNames;
AnchoredSetVariable m_variableFilesTmpNames;
AnchoredSetVariable m_variableMultipartPartHeaders;
int m_variableOffset;

3394
src/parser/seclang-parser.cc
View File

File diff suppressed because it is too large Load Diff

1342
src/parser/seclang-parser.hh
View File

File diff suppressed because one or more lines are too long

14
src/parser/seclang-parser.yy
View File

@ -217,6 +217,7 @@ class Driver;
#include "src/variables/request_body_length.h"
#include "src/variables/request_cookies.h"
#include "src/variables/request_cookies_names.h"
#include "src/variables/multipart_part_headers.h"
#include "src/variables/request_file_name.h"
#include "src/variables/request_headers.h"
#include "src/variables/request_headers_names.h"
@ -352,6 +353,7 @@ using namespace modsecurity::operators;
VARIABLE_RESPONSE_HEADERS
VARIABLE_GEO
VARIABLE_REQUEST_COOKIES_NAMES
VARIABLE_MULTIPART_PART_HEADERS
VARIABLE_ARGS_COMBINED_SIZE
VARIABLE_ARGS_GET_NAMES
VARIABLE_RULE
@ -2061,6 +2063,18 @@ var:
{
VARIABLE_CONTAINER($$, new variables::RequestCookiesNames_NoDictElement());
}
| VARIABLE_MULTIPART_PART_HEADERS DICT_ELEMENT
{
VARIABLE_CONTAINER($$, new variables::MultipartPartHeaders_DictElement($2));
}
| VARIABLE_MULTIPART_PART_HEADERS DICT_ELEMENT_REGEXP
{
VARIABLE_CONTAINER($$, new variables::MultipartPartHeaders_DictElementRegexp($2));
}
| VARIABLE_MULTIPART_PART_HEADERS
{
VARIABLE_CONTAINER($$, new variables::MultipartPartHeaders_NoDictElement());
}
| VARIABLE_RULE DICT_ELEMENT
{
VARIABLE_CONTAINER($$, new variables::Rule_DictElement($2));

7525
src/parser/seclang-scanner.cc
View File

File diff suppressed because it is too large Load Diff

3
src/parser/seclang-scanner.ll
View File

@ -253,6 +253,7 @@ VARIABLE_REQUEST_HEADERS (?i:REQUEST_HEADERS)
VARIABLE_RESPONSE_HEADERS (?i:RESPONSE_HEADERS)
VARIABLE_GEO (?i:GEO)
VARIABLE_REQUEST_COOKIES_NAMES (?i:REQUEST_COOKIES_NAMES)
VARIABLE_MULTIPART_PART_HEADERS (?i:MULTIPART_PART_HEADERS)
VARIABLE_RULE (?i:RULE)
VARIABLE_SESSION (?i:(SESSION))
VARIABLE_IP (?i:(IP))
@ -996,6 +997,8 @@ EQUALS_MINUS (?i:=\-)
{VARIABLE_GEO}[:.] { BEGINX(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_GEO(*driver.loc.back()); }
{VARIABLE_REQUEST_COOKIES_NAMES} { return p::make_VARIABLE_REQUEST_COOKIES_NAMES(*driver.loc.back()); }
{VARIABLE_REQUEST_COOKIES_NAMES}[:.] { BEGINX(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_REQUEST_COOKIES_NAMES(*driver.loc.back()); }
{VARIABLE_MULTIPART_PART_HEADERS} { return p::make_VARIABLE_MULTIPART_PART_HEADERS(*driver.loc.back()); }
{VARIABLE_MULTIPART_PART_HEADERS}[:.] { BEGINX(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_MULTIPART_PART_HEADERS(*driver.loc.back()); }
{VARIABLE_RULE} { return p::make_VARIABLE_RULE(*driver.loc.back()); }
{VARIABLE_RULE}[:.] { BEGINX(EXPECTING_VAR_PARAMETER); return p::make_VARIABLE_RULE(*driver.loc.back()); }
{VARIABLE_FILES_TMP_NAMES} { return p::make_VARIABLE_FILES_TMP_NAMES(*driver.loc.back()); }

91
src/request_body_processor/multipart.cc
View File

@ -102,8 +102,11 @@ Multipart::Multipart(const std::string &header, Transaction *transaction)
m_bufptr(NULL),
m_bufleft(0),
m_buf_offset(0),
m_crlf_state(0),
m_crlf_state_buf_end(0),
m_mpp(NULL),
m_mpp_state(0),
m_mpp_substate_part_data_read(0),
m_reserve{0},
m_seen_data(0),
m_is_complete(0),
@ -504,6 +507,8 @@ int Multipart::process_part_data(std::string *error, size_t offset) {
char localreserve[2] = { '\0', '\0' }; /* initialized to quiet warning */
int bytes_reserved = 0;
m_mpp_substate_part_data_read = 1;
/* Preserve some bytes for later. */
if (((MULTIPART_BUF_SIZE - m_bufleft) >= 1) && (*(p - 1) == '\n')) {
if (((MULTIPART_BUF_SIZE - m_bufleft) >= 2) && (*(p - 2) == '\r')) {
@ -697,6 +702,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
/* Check for nul bytes. */
len = MULTIPART_BUF_SIZE - m_bufleft;
int len_without_termination = len - 1;
for (i = 0; i < len; i++) {
if (m_buf[i] == '\0') {
ms_dbg_a(m_transaction, 1,
@ -714,6 +720,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
if (len > 1) {
if (m_buf[len - 2] == '\r') {
m_flag_crlf_line = 1;
len_without_termination--;
} else {
m_flag_lf_line = 1;
}
@ -727,6 +734,13 @@ int Multipart::process_part_header(std::string *error, int offset) {
std::string header_value("");
int rc;
/* record the previous completed header */
if (!m_mpp->m_last_header_line.empty()) {
m_mpp->m_header_lines.push_back(std::make_pair(
offset-m_mpp->m_last_header_line.length(), m_mpp->m_last_header_line));
m_mpp->m_last_header_line.assign("");
}
if (m_mpp->m_headers.count("Content-Disposition") == 0) {
ms_dbg_a(m_transaction, 1,
"Multipart: Part missing Content-Disposition header.");
@ -787,6 +801,7 @@ int Multipart::process_part_header(std::string *error, int offset) {
}
m_mpp_state = 1;
m_mpp_substate_part_data_read = 0;
m_mpp->m_last_header_name.assign("");
} else { /* Header line. */
if (isspace(m_buf[0])) {
@ -848,12 +863,21 @@ int Multipart::process_part_header(std::string *error, int offset) {
error->assign("Multipart: Part header too long.");
return false;
}
m_mpp->m_last_header_line = m_mpp->m_last_header_name + ": " + new_value;
} else {
char *data;
std::string header_value;
std::string header_name;
/* new header */
/* record the previous completed header */
if (!m_mpp->m_last_header_line.empty()) {
m_mpp->m_header_lines.push_back(std::make_pair(
offset-m_mpp->m_last_header_line.length(), m_mpp->m_last_header_line));
m_mpp->m_last_header_line.assign("");
}
data = m_buf;
while ((*data != ':') && (*data != '\0')) {
data++;
@ -910,6 +934,11 @@ int Multipart::process_part_header(std::string *error, int offset) {
ms_dbg_a(m_transaction, 9,
"Multipart: Added part header \"" + header_name \
+ "\" \"" + header_value + "\".");
if (len_without_termination > 0) {
m_mpp->m_last_header_line.assign(m_buf);
} else {
m_mpp->m_last_header_line.assign("");
}
}
}
@ -920,6 +949,13 @@ int Multipart::process_part_header(std::string *error, int offset) {
int Multipart::process_boundary(int last_part) {
/* if there was a part being built finish it */
if (m_mpp != NULL) {
/* record all the part header lines from the part into the transaction collection */
for (const auto& header_line : m_mpp->m_header_lines) {
m_transaction->m_variableMultipartPartHeaders.set(m_mpp->m_name,
header_line.second, header_line.first);
ms_dbg_a(m_transaction, 9, "Multipart: Added part header line:" + header_line.second );
}
/* close the temp file */
if ((m_mpp->m_type == MULTIPART_FILE) && (m_mpp->m_tmp_file)
&& (m_mpp->m_tmp_file->isValid())) {
@ -972,6 +1008,7 @@ int Multipart::process_boundary(int last_part) {
m_mpp = new MultipartPart();
m_mpp_state = 0;
m_mpp_substate_part_data_read = 0;
m_reserve[0] = 0;
m_reserve[1] = 0;
@ -1099,6 +1136,33 @@ int Multipart::multipart_complete(std::string *error) {
m_boundary.size()) == 0)
&& (*(m_buf + 2 + m_boundary.size()) == '-')
&& (*(m_buf + 2 + m_boundary.size() + 1) == '-')) {
// these next two checks may result in repeating work from earlier in this fn
// ignore the duplication for now to minimize refactoring
if ((m_crlf_state_buf_end == 2) && (m_flag_lf_line != 1)) {
m_flag_lf_line = 1;
m_transaction->m_variableMultipartLFLine.set(std::to_string(m_flag_lf_line),
m_transaction->m_variableOffset);
m_transaction->m_variableMultipartCrlfLFLines.set(std::to_string(m_flag_crlf_line && m_flag_lf_line),
m_transaction->m_variableOffset);
if (m_flag_crlf_line && m_flag_lf_line) {
ms_dbg_a(m_transaction, 4, "Multipart: Warning: mixed line endings used (CRLF/LF).");
} else if (m_flag_lf_line) {
ms_dbg_a(m_transaction, 4, "Multipart: Warning: incorrect line endings used (LF).");
}
m_transaction->m_variableMultipartStrictError.set(
std::to_string(m_flag_lf_line) , m_transaction->m_variableOffset);
}
if ((m_mpp_substate_part_data_read == 0) && (m_flag_invalid_part != 1)) {
// it looks like the final boundary, but it's where part data should begin
m_flag_invalid_part = 1;
ms_dbg_a(m_transaction, 3, "Multipart: Invalid part (data contains final boundary)");
m_transaction->m_variableMultipartStrictError.set(
std::to_string(m_flag_invalid_part) , m_transaction->m_variableOffset);
m_transaction->m_variableMultipartInvalidPart.set(std::to_string(m_flag_invalid_part),
m_transaction->m_variableOffset);
ms_dbg_a(m_transaction, 4, "Multipart: Warning: invalid part parsing.");
}
/* Looks like the final boundary - process it. */
if (process_boundary(1 /* final */) < 0) {
m_flag_error = 1;
@ -1493,6 +1557,15 @@ bool Multipart::process(const std::string& data, std::string *error,
if ((strlen(m_buf) >= m_boundary.size() + 2)
&& (strncmp(m_buf + 2, m_boundary.c_str(),
m_boundary.size()) == 0)) {
if (m_crlf_state_buf_end == 2) {
m_flag_lf_line = 1;
}
if ((m_mpp_substate_part_data_read == 0) && (m_boundary_count > 0)) {
/* string matches our boundary, but it's where part data should begin */
m_flag_invalid_part = 1;
ms_dbg_a(m_transaction, 3, "Multipart: Invalid part (data contains boundary)");
} else {
char *boundary_end = m_buf + 2 + m_boundary.size();
/* if it match, AND there was a matched boundary at least,
set the m_flag_unmatched_boundary flag to 2
@ -1558,6 +1631,7 @@ bool Multipart::process(const std::string& data, std::string *error,
+ std::string(m_buf));
return false;
}
}
} else { /* It looks like a boundary but */
/* we couldn't match it. */
char *p = NULL;
@ -1667,7 +1741,24 @@ bool Multipart::process(const std::string& data, std::string *error,
m_bufptr = m_buf;
m_bufleft = MULTIPART_BUF_SIZE;
m_buf_contains_line = (c == 0x0a) ? 1 : 0;
if (c == 0x0a) {
if (m_crlf_state == 1) {
m_crlf_state = 3;
} else {
m_crlf_state = 2;
}
}
m_crlf_state_buf_end = m_crlf_state;
}
if (c == 0x0d) {
m_crlf_state = 1;
} else if (c != 0x0a) {
m_crlf_state = 0;
}
if ((m_is_complete) && (inleft != 0)) {
m_flag_data_after = 1;

20
src/request_body_processor/multipart.h
View File

@ -138,6 +138,9 @@ class MultipartPart {
std::string m_last_header_name;
std::unordered_map<std::string, std::pair<size_t, std::string>,
MyHash, MyEqual> m_headers;
std::string m_last_header_line;
std::vector<std::pair<size_t, std::string>> m_header_lines;
unsigned int m_offset;
unsigned int m_length;
@ -186,6 +189,15 @@ class Multipart {
unsigned int m_buf_offset;
/* line ending status seen immediately before current position.
* 0 = neither LF nor CR; 1 = prev char CR; 2 = prev char LF alone;
* 3 = prev two chars were CRLF
*/
int m_crlf_state;
/* crlf_state at end of previous buffer */
int m_crlf_state_buf_end;
/* pointer that keeps track of a part while
* it is being built
*/
@ -197,6 +209,14 @@ class Multipart {
*/
int m_mpp_state;
/* part parsing substate; if mpp_state is 1 (collecting
* data), then for this variable:
* 0 means we have not yet read any data between the
* post-headers blank line and the next boundary
* 1 means we have read at some data after that blank line
*/
int m_mpp_substate_part_data_read;
/* because of the way this parsing algorithm
* works we hold back the last two bytes of
* each data chunk so that we can discard it

41
src/variables/multipart_part_headers.h Normal file
View File

@ -0,0 +1,41 @@
/*
* ModSecurity, http://www.modsecurity.org/
* Copyright (c) 2022 Trustwave Holdings, Inc. (http://www.trustwave.com/)
*
* You may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* If any of the files related to licensing are missing or if you have any
* other questions related to licensing please contact Trustwave Holdings, Inc.
* directly using the email address security@modsecurity.org.
*
*/
#include <iostream>
#include <string>
#include <vector>
#include <list>
#include <utility>
#ifndef SRC_VARIABLES_MULTIPART_PART_HEADERS_H_
#define SRC_VARIABLES_MULTIPART_PART_HEADERS_H_
#include "src/variables/variable.h"
namespace modsecurity {
class Transaction;
namespace variables {
DEFINE_VARIABLE_DICT(MultipartPartHeaders, MULTIPART_PART_HEADERS,
m_variableMultipartPartHeaders)
} // namespace variables
} // namespace modsecurity
#endif // SRC_VARIABLES_MULTIPART_PART_HEADERS_H_

2
test/cppcheck_suppressions.txt
View File

@ -54,7 +54,7 @@ duplicateBranch:src/request_body_processor/multipart.cc:93
danglingTempReference:src/modsecurity.cc:206
knownConditionTrueFalse:src/operators/validate_url_encoding.cc:77
knownConditionTrueFalse:src/operators/verify_svnr.cc:87
rethrowNoCurrentException:headers/modsecurity/transaction.h:307
rethrowNoCurrentException:headers/modsecurity/transaction.h:309
rethrowNoCurrentException:src/rule_with_actions.cc:123
ctunullpointer:src/rule_with_actions.cc:237
ctunullpointer:src/rule_with_operator.cc:135

167
test/test-cases/regression/variable-MULTIPART_PART_HEADERS.json Normal file
View File

@ -0,0 +1,167 @@
[
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MULTIPART_PART_HEADERS (all headers)",
"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=-----------------------------69343412719991675451336310646",
"Expect":"100-continue"
},
"uri":"/",
"method":"POST",
"body":[
"-------------------------------69343412719991675451336310646",
"Content-Disposition: form-data; name=parm1",
"Content-Type: image/jpeg",
"",
"1",
"-------------------------------69343412719991675451336310646",
"Content-Disposition: form-data; name=parm2",
"",
"2",
"-------------------------------69343412719991675451336310646--"
]
},
"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":{
"http_code": 403,
"debug_log":"Variable: MULTIPART_PART_HEADERS:parm1.*Rule returned 1"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_PART_HEADERS \"@rx content-type:.*jpeg\" \"phase:2,deny,status:403,id:500074,t:lowercase\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MULTIPART_PART_HEADERS (specific header - 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":"330",
"Content-Type":"multipart/form-data; boundary=-----------------------------69343412719991675451336310646",
"Expect":"100-continue"
},
"uri":"/",
"method":"POST",
"body":[
"-------------------------------69343412719991675451336310646",
"Content-Disposition: form-data; name=parm1",
"Content-Type: image/jpeg",
"",
"1",
"-------------------------------69343412719991675451336310646",
"Content-Disposition: form-data; name=parm2",
"",
"2",
"-------------------------------69343412719991675451336310646--"
]
},
"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":{
"http_code": 403,
"debug_log":"Variable: MULTIPART_PART_HEADERS:parm1.*Rule returned 1"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_PART_HEADERS:parm1 \"@rx content-type:.*jpeg\" \"phase:2,deny,status:403,id:500074,t:lowercase\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MULTIPART_PART_HEADERS (specific header - no 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":"330",
"Content-Type":"multipart/form-data; boundary=-----------------------------69343412719991675451336310646",
"Expect":"100-continue"
},
"uri":"/",
"method":"POST",
"body":[
"-------------------------------69343412719991675451336310646",
"Content-Disposition: form-data; name=parm1",
"Content-Type: image/jpeg",
"",
"1",
"-------------------------------69343412719991675451336310646",
"Content-Disposition: form-data; name=parm2",
"",
"2",
"-------------------------------69343412719991675451336310646--"
]
},
"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":{
"http_code": 200
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_PART_HEADERS:parm2 \"@rx content-type:.*jpeg\" \"phase:2,deny,status:403,id:500074,t:lowercase\""
]
}
]