Multipart Content-Disposition should allow filename* field

This commit is contained in:
martinhsv 2019-12-05 12:51:22 -08:00 committed by Felipe Zimmerle
parent 1b1fdc055b
commit 136db3e582
No known key found for this signature in database
GPG Key ID: E6DFB08CE8B11277
3 changed files with 374 additions and 23 deletions

View File

@ -1,6 +1,8 @@
v3.x.y - YYYY-MMM-DD (to be released)
-------------------------------------
- Multipart Content-Dispostion should allow field: filename*=
[@martinhsv]
- Fix rule-update-target for non-regex
[Issue 2251 - @martinhsv]
- Fix configure script when packaging for Buildroot

View File

@ -35,6 +35,8 @@
namespace modsecurity {
namespace RequestBodyProcessor {
static const char* mime_charset_special = "!#$%&+-^_`{}~";
static const char* attr_char_special = "!#$&+-.^_`~";
Multipart::Multipart(const std::string &header, Transaction *transaction)
: m_reqbody_no_files_length(0),
@ -208,6 +210,7 @@ void Multipart::validate_quotes(const char *data) {
int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
const char *p = NULL;
std::string filenameStar;
/* accept only what we understand */
if (strncmp(c_d_value, "form-data", 9) != 0) {
@ -276,13 +279,45 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
return -6;
}
/* Accept both quotes as some backends will accept them, but
* technically "'" is invalid and so flag_invalid_quoting is
* set so the user can deal with it in the rules if they so wish.
*/
if (name == "filename*") {
/* filename*=charset'[optional-language]'filename */
/* Read beyond the charset and the optional language*/
const char* start_of_charset = p;
while ((*p != '\0') && (isalnum(*p) || (strchr(mime_charset_special, *p)))) {
p++;
}
if ((*p != '\'') || (p == start_of_charset)) {
return -16; // Must be at least one legit char before ' for start of language
}
p++;
while ((*p != '\0') && (*p != '\'')) {
p++;
}
if (*p != '\'') {
return -17; // Single quote for end-of-language not found
}
p++;
if ((*p == '"') || (*p == '\'')) {
/* quoted */
/* Now read what should be the actual filename */
const char* start_of_filename = p;
while ((*p != '\0') && (*p != ';')) {
if (*p == '%') {
if ((*(p+1) == '\0') || (!isxdigit(*(p+1))) || (!isxdigit(*p+2))) {
return -18;
}
p += 3;
} else if (isalnum(*p) || strchr(attr_char_special, *p)) {
p++;
} else {
return -19;
}
}
value.assign(start_of_filename, p - start_of_filename);
} else if ((*p == '"') || (*p == '\'')) {
/* Accept both quotes as some backends will accept them, but
* 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;
if (quote == '\'') {
@ -364,8 +399,17 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
m_mpp->m_filenameOffset = offset + ((p - c_d_value) - value.size());
ms_dbg_a(m_transaction, 9,
"Multipart: Content-Disposition filename: " \
+ value + ".");
"Multipart: Content-Disposition filename: " + value + ".");
} else if (name == "filename*") {
if (!filenameStar.empty()) {
ms_dbg_a(m_transaction, 4,
"Multipart: Warning: Duplicate Content-Disposition " \
"filename*: " + value + ".");
return -20;
}
filenameStar.assign(value);
ms_dbg_a(m_transaction, 9,
"Multipart: Content-Disposition filename*: " + value + ".");
} else {
return -11;
}
@ -377,26 +421,34 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
/* the next character must be a zero or a semi-colon */
if (*p == '\0') {
return 1; /* this is OK */
}
if (*p != ';') {
p--;
if (*p == '\'' || *p == '\"') {
ms_dbg_a(m_transaction, 9,
"Multipart: Invalid quoting detected: " \
+ std::string(p) + " length " \
+ std::to_string(strlen(p)) + " bytes");
m_flag_invalid_quoting = 1;
// Just let the 'while' condition end the loop
} else {
if (*p != ';') {
p--;
if (*p == '\'' || *p == '\"') {
ms_dbg_a(m_transaction, 9,
"Multipart: Invalid quoting detected: " \
+ std::string(p) + " length " \
+ std::to_string(strlen(p)) + " bytes");
m_flag_invalid_quoting = 1;
}
p++;
return -12;
}
p++;
return -12;
p++; /* move over the semi-colon */
}
p++; /* move over the semi-colon */
}
/* loop will stop when (*p == '\0') */
}
if (!filenameStar.empty() && m_mpp->m_filename.empty()) {
ms_dbg_a(m_transaction, 4,
"Multipart: Warning: no filename= but filename*:" \
+ filenameStar + ".");
return -21;
}
return 1;
}
@ -675,8 +727,14 @@ int Multipart::process_part_header(std::string *error, int offset) {
}
header_value = m_mpp->m_headers.at("Content-Disposition").second;
rc = parse_content_disposition(header_value.c_str(),
m_mpp->m_headers.at("Content-Disposition").first);
try {
rc = parse_content_disposition(header_value.c_str(),
m_mpp->m_headers.at("Content-Disposition").first);
} catch (...) {
ms_dbg_a(m_transaction, 1,
"Multipart: Unexpected error parsing Content-Disposition header.");
rc = -99;
}
if (rc < 0) {
ms_dbg_a(m_transaction, 1,
"Multipart: Invalid Content-Disposition header ("

View File

@ -0,0 +1,291 @@
[
{
"enabled":1,
"version_min":300000,
"title":"multipart Content-Disposition should allow filename* field (1/6)",
"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=\"filedata\"; filename=\"03CB1664.txt\"; filename*=utf-8''03CB1664.txt",
"Content-Type: text/plain",
"",
"This is a very small test file..",
"----------------------------756b6d74fa1a8ee2--"
]
},
"response":{
"headers":"",
"body":""
},
"expected":{
"debug_log":"Target value: \"03CB1664.txt\" \\(Variable: MULTIPART_FILENAME"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_FILENAME \"@contains 0\" \"id:1,phase:2,pass,t:trim\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"multipart Content-Disposition should allow filename* field (2/6)",
"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=\"filedata\"; filename*= ISO-8859-1''ab0-_xy.txt; filename=\"ab0-_xy.txt\"",
"Content-Type: text/plain",
"",
"This is a very small test file..",
"----------------------------756b6d74fa1a8ee2--"
]
},
"response":{
"headers":"",
"body":""
},
"expected":{
"debug_log":"Target value: \"ab0-_xy.txt\" \\(Variable: MULTIPART_FILENAME"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_FILENAME \"@contains 0\" \"id:1,phase:2,pass,t:trim\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"multipart Content-Disposition should allow filename* field (3/6)",
"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=\"filedata\"; filename*=utf-8''03CB1664.txt",
"Content-Type: text/plain",
"",
"This is a very small test file..",
"----------------------------756b6d74fa1a8ee2--\r"
]
},
"response":{
"headers":"",
"body":""
},
"expected":{
"debug_log":"Warning: no filename= but filename*"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_FILENAME \"@contains 0\" \"id:1,phase:2,pass,t:trim\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"multipart Content-Disposition should allow filename* field (4/6)",
"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=\"filedata\"; filename=\"03CB1664.txt\"; filename*=''03CB1664.txt",
"Content-Type: text/plain",
"",
"This is a very small test file..",
"----------------------------756b6d74fa1a8ee2--"
]
},
"response":{
"headers":"",
"body":""
},
"expected":{
"debug_log":"Multipart: Invalid Content-Disposition header \\(-16"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_FILENAME \"@contains 0\" \"id:1,phase:2,pass,t:trim\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"multipart Content-Disposition should allow filename* field (5/6)",
"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=\"filedata\"; filename=\"03CB1664.txt\"; filename*=UTF-8'03CB1664.txt",
"Content-Type: text/plain",
"",
"This is a very small test file..",
"----------------------------756b6d74fa1a8ee2--"
]
},
"response":{
"headers":"",
"body":""
},
"expected":{
"debug_log":"Multipart: Invalid Content-Disposition header \\(-17"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_FILENAME \"@contains 0\" \"id:1,phase:2,pass,t:trim\""
]
},
{
"enabled":1,
"version_min":300000,
"title":"multipart Content-Disposition should allow filename* field (6/6)",
"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=\"filedata\"; filename=\"03CB1664.txt\"; filename*=utf-8''%61%4G.txt",
"Content-Type: text/plain",
"",
"This is a very small test file..",
"----------------------------756b6d74fa1a8ee2--"
]
},
"response":{
"headers":"",
"body":""
},
"expected":{
"debug_log":"Multipart: Invalid Content-Disposition header \\(-18"
},
"rules":[
"SecRuleEngine On",
"SecRule MULTIPART_FILENAME \"@contains 0\" \"id:1,phase:2,pass,t:trim\""
]
}
]