Skip to content

Commit 52958fa

Browse files
martinhsvmartinhsv
authored
Merge pull request #2661 from martinhsv/v3/master
Multipart names may include single quote if double-quote enclosed
2 parents c072ac2 + f34b49f commit 52958fa

File tree

4 files changed

+134
-5
lines changed

4 files changed

+134
-5
lines changed

Diff for: ‎CHANGES

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
v3.x.y - YYYY-MMM-DD (to be released)
22
-------------------------------------
33

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

Diff for: ‎src/request_body_processor/multipart.cc

+11-4
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,18 @@ int Multipart::boundary_characters_valid(const char *boundary) {
227227
}
228228

229229

230-
void Multipart::validate_quotes(const char *data) {
230+
void Multipart::validate_quotes(const char *data, char quote) {
231231
int i, len;
232232

233233
if (data == NULL)
234234
return;
235235

236+
// if the value was enclosed in double quotes, then we don't care about
237+
// a single quote character within the name.
238+
if (quote == '"') {
239+
return;
240+
}
241+
236242
len = strlen(data);
237243

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

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

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

409416
/* evaluate part */
410417
if (name == "name") {
411-
validate_quotes(value.c_str());
418+
validate_quotes(value.c_str(), quote);
412419

413420
m_transaction->m_variableMultipartName.set(value, value,
414421
offset + ((p - c_d_value) - value.size()));
@@ -424,7 +431,7 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
424431
ms_dbg_a(m_transaction, 9,
425432
"Multipart: Content-Disposition name: " + value + ".");
426433
} else if (name == "filename") {
427-
validate_quotes(value.c_str());
434+
validate_quotes(value.c_str(), quote);
428435
m_transaction->m_variableMultipartFileName.set(value, value, \
429436
offset + ((p - c_d_value) - value.size()));
430437

Diff for: ‎src/request_body_processor/multipart.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ class Multipart {
162162
int process_part_header(std::string *error, int offset);
163163
int process_part_data(std::string *error, size_t offset);
164164

165-
void validate_quotes(const char *data);
165+
void validate_quotes(const char *data, char quote);
166166

167167
size_t m_reqbody_no_files_length;
168168
std::list<MultipartPart *> m_parts;

Diff for: ‎test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json

+120
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,126 @@
342342
"SecRuleEngine On",
343343
"SecRule REQBODY_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\""
344344
]
345+
},
346+
{
347+
"enabled":1,
348+
"version_min":300000,
349+
"title":"Testing Variables :: MULTIPART_STRICT_ERROR - IQ ",
350+
"client":{
351+
"ip":"200.249.12.31",
352+
"port":123
353+
},
354+
"server":{
355+
"ip":"200.249.12.31",
356+
"port":80
357+
},
358+
"request":{
359+
"headers":{
360+
"Host":"localhost",
361+
"User-Agent":"curl/7.38.0",
362+
"Accept":"*/*",
363+
"Content-Length":"330",
364+
"Content-Type":"multipart/form-data; boundary=--------------------------756b6d74fa1a8ee2",
365+
"Expect":"100-continue"
366+
},
367+
"uri":"/",
368+
"method":"POST",
369+
"body":[
370+
"----------------------------756b6d74fa1a8ee2",
371+
"Content-Disposition: form-data; name=\"name\"",
372+
"",
373+
"test",
374+
"----------------------------756b6d74fa1a8ee2",
375+
"Content-Disposition: form-data; name=file'data; filename=\"small_text_file.txt\"",
376+
"Content-Type: text/plain",
377+
"",
378+
"This is a very small test file..",
379+
"----------------------------756b6d74fa1a8ee2",
380+
"Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"",
381+
"Content-Type: text/plain",
382+
"",
383+
"This is another very small test file..",
384+
"----------------------------756b6d74fa1a8ee2--"
385+
]
386+
},
387+
"response":{
388+
"headers":{
389+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
390+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
391+
"Content-Type":"text/html"
392+
},
393+
"body":[
394+
"no need."
395+
]
396+
},
397+
"expected":{
398+
"http_code": 403,
399+
"debug_log":"Warning: invalid quoting used"
400+
},
401+
"rules":[
402+
"SecRuleEngine On",
403+
"SecRule MULTIPART_STRICT_ERROR \"!@eq 0\" \"id:1,phase:2,deny,status:403\""
404+
]
405+
},
406+
{
407+
"enabled":1,
408+
"version_min":300000,
409+
"title":"Testing Variables :: MULTIPART_STRICT_ERROR - IQ ",
410+
"client":{
411+
"ip":"200.249.12.31",
412+
"port":123
413+
},
414+
"server":{
415+
"ip":"200.249.12.31",
416+
"port":80
417+
},
418+
"request":{
419+
"headers":{
420+
"Host":"localhost",
421+
"User-Agent":"curl/7.38.0",
422+
"Accept":"*/*",
423+
"Content-Length":"330",
424+
"Content-Type":"multipart/form-data; boundary=--------------------------756b6d74fa1a8ee2",
425+
"Expect":"100-continue"
426+
},
427+
"uri":"/",
428+
"method":"POST",
429+
"body":[
430+
"----------------------------756b6d74fa1a8ee2",
431+
"Content-Disposition: form-data; name=\"name\"",
432+
"",
433+
"test",
434+
"----------------------------756b6d74fa1a8ee2",
435+
"Content-Disposition: form-data; name=\"file'data\"; filename=\"small_text_file.txt\"",
436+
"Content-Type: text/plain",
437+
"",
438+
"This is a very small test file..",
439+
"----------------------------756b6d74fa1a8ee2",
440+
"Content-Disposition: form-data; name=\"filedata\"; filename=\"small_text_file.txt\"",
441+
"Content-Type: text/plain",
442+
"",
443+
"This is another very small test file..",
444+
"----------------------------756b6d74fa1a8ee2--"
445+
]
446+
},
447+
"response":{
448+
"headers":{
449+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
450+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
451+
"Content-Type":"text/html"
452+
},
453+
"body":[
454+
"no need."
455+
]
456+
},
457+
"expected":{
458+
"http_code": 200
459+
},
460+
"rules":[
461+
"SecRuleEngine On",
462+
"SecRule MULTIPART_INVALID_QUOTING \"!@eq 0\" \"id:1,phase:2,deny,status:403\"",
463+
"SecRule MULTIPART_STRICT_ERROR \"!@eq 0\" \"id:2,phase:2,pass\""
464+
]
345465
}
346466
]
347467

0 commit comments

Comments
 (0)