Skip to content

Multipart names may include single quote if double-quote enclosed #2661

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 23, 2021
Merged

Multipart names may include single quote if double-quote enclosed #2661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
v3.x.y - YYYY-MMM-DD (to be released)
-------------------------------------

- Multipart names/filenames may include single quote if double-quote enclosed
[Issue #2352 @martinhsv]
- Add SecRequestBodyJsonDepthLimit to modsecurity.conf-recommended
[Issue #2647 @theMiddleBlue, @airween, @877509395 ,@martinhsv]

Expand Down
15 changes: 11 additions & 4 deletions src/request_body_processor/multipart.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,12 +227,18 @@ int Multipart::boundary_characters_valid(const char *boundary) {
}


void Multipart::validate_quotes(const char *data) {
void Multipart::validate_quotes(const char *data, char quote) {
int i, len;

if (data == NULL)
return;

// if the value was enclosed in double quotes, then we don't care about
// a single quote character within the name.
if (quote == '"') {
return;
}

len = strlen(data);

for (i = 0; i < len; i++) {
Expand Down Expand Up @@ -318,6 +324,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
return -6;
}

char quote = '\0';
if (name == "filename*") {
/* filename*=charset'[optional-language]'filename */
/* Read beyond the charset and the optional language*/
Expand Down Expand Up @@ -357,7 +364,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
* technically "'" is invalid and so flag_invalid_quoting is
* set so the user can deal with it in the rules if they so wish.
*/
char quote = *p;
quote = *p; // remember which quote character was used for the value

if (quote == '\'') {
m_flag_invalid_quoting = 1;
Expand Down Expand Up @@ -408,7 +415,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {

/* evaluate part */
if (name == "name") {
validate_quotes(value.c_str());
validate_quotes(value.c_str(), quote);

m_transaction->m_variableMultipartName.set(value, value,
offset + ((p - c_d_value) - value.size()));
Expand All @@ -424,7 +431,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
ms_dbg_a(m_transaction, 9,
"Multipart: Content-Disposition name: " + value + ".");
} else if (name == "filename") {
validate_quotes(value.c_str());
validate_quotes(value.c_str(), quote);
m_transaction->m_variableMultipartFileName.set(value, value, \
offset + ((p - c_d_value) - value.size()));

Expand Down
2 changes: 1 addition & 1 deletion src/request_body_processor/multipart.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ class Multipart {
int process_part_header(std::string *error, int offset);
int process_part_data(std::string *error, size_t offset);

void validate_quotes(const char *data);
void validate_quotes(const char *data, char quote);

size_t m_reqbody_no_files_length;
std::list<MultipartPart *> m_parts;
Expand Down
120 changes: 120 additions & 0 deletions test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,126 @@
"SecRuleEngine On",
"SecRule REQBODY_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MULTIPART_STRICT_ERROR - IQ ",
"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":"/",
"method":"POST",
"body":[
"----------------------------756b6d74fa1a8ee2",
"Content-Disposition: form-data; name=\"name\"",
"",
"test",
"----------------------------756b6d74fa1a8ee2",
"Content-Disposition: form-data; name=file'data; 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":{
"http_code": 403,
"debug_log":"Warning: invalid quoting used"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_STRICT_ERROR \"!@eq 0\" \"id:1,phase:2,deny,status:403\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"Testing Variables :: MULTIPART_STRICT_ERROR - IQ ",
"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":"/",
"method":"POST",
"body":[
"----------------------------756b6d74fa1a8ee2",
"Content-Disposition: form-data; name=\"name\"",
"",
"test",
"----------------------------756b6d74fa1a8ee2",
"Content-Disposition: form-data; name=\"file'data\"; 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":{
"http_code": 200
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_INVALID_QUOTING \"!@eq 0\" \"id:1,phase:2,deny,status:403\"",
"SecRule MULTIPART_STRICT_ERROR \"!@eq 0\" \"id:2,phase:2,pass\""
]
}
]