Skip to content

Commit fbf1569

Browse files
authored
Merge pull request owasp-modsecurity#3546 from airween/v3/multipartbofix
fix: buffer overflow in multipart body proc
2 parents 46bfa52 + 3b8b094 commit fbf1569

File tree

2 files changed

+59
-3
lines changed

2 files changed

+59
-3
lines changed

src/request_body_processor/multipart.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -362,11 +362,11 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
362362
const char* start_of_filename = p;
363363
while ((*p != '\0') && (*p != ';')) {
364364
if (*p == '%') {
365-
if ((*(p+1) == '\0') || (!isxdigit(*(p+1))) || (!isxdigit(*(p+2)))) {
365+
if ((*(p+1) == '\0') || (*(p+2) == '\0') || (!isxdigit(static_cast<unsigned char>(*(p+1)))) || (!isxdigit(static_cast<unsigned char>(*(p+2))))) {
366366
return -18;
367367
}
368368
p += 3;
369-
} else if (isalnum(*p) || strchr(attr_char_special, *p)) {
369+
} else if (isalnum(static_cast<unsigned char>(*p)) || strchr(attr_char_special, *p)) {
370370
p++;
371371
} else {
372372
return -19;
@@ -415,7 +415,12 @@ int Multipart::parse_content_disposition(const char *c_d_value, int offset) {
415415
value.append((p++), 1);
416416
}
417417

418-
p++; /* go over the quote at the end */
418+
if (*p == quote) {
419+
p++; /* go over the quote at the end */
420+
} else {
421+
m_flag_invalid_quoting = 1;
422+
return -15; /* closing quote not found */
423+
}
419424

420425
} else {
421426
/* not quoted */

test/test-cases/regression/request-body-parser-multipart.json

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3417,5 +3417,56 @@
34173417
"SecruleEngine On",
34183418
"SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:403,id:500077\""
34193419
]
3420+
},
3421+
{
3422+
"enabled": 1,
3423+
"version_min": 300000,
3424+
"title": "multipart parser (invalid part header - missing trailing quote)",
3425+
"client": {
3426+
"ip": "200.249.12.31",
3427+
"port": 123
3428+
},
3429+
"server": {
3430+
"ip": "200.249.12.31",
3431+
"port": 80
3432+
},
3433+
"request": {
3434+
"headers": {
3435+
"Host": "localhost",
3436+
"User-Agent": "curl/7.38.0",
3437+
"Accept": "*/*",
3438+
"Content-Length": "145",
3439+
"Content-Type": "multipart/form-data; boundary=a",
3440+
"Expect": "100-continue"
3441+
},
3442+
"uri": "/",
3443+
"method": "POST",
3444+
"body": [
3445+
"--a\r\n",
3446+
"Content-Disposition: form-data; name=\"file\"; filename=\"1.jsp\r\n",
3447+
"\r\n",
3448+
"Some content\r\n",
3449+
"--a--\r\n"
3450+
]
3451+
},
3452+
"response": {
3453+
"headers": {
3454+
"Date": "Mon, 13 Jul 2015 20:02:41 GMT",
3455+
"Last-Modified": "Sun, 26 Oct 2014 22:33:37 GMT",
3456+
"Content-Type": "text/html",
3457+
"Content-Length": "8"
3458+
},
3459+
"body": [
3460+
"no need."
3461+
]
3462+
},
3463+
"expected": {
3464+
"debug_log": "Multipart: Invalid Content-Disposition header \\(-15\\): form-data; name=\"file\"; filename=\"1.jsp",
3465+
"http_code": 403
3466+
},
3467+
"rules": [
3468+
"SecruleEngine On",
3469+
"SecRule REQBODY_PROCESSOR_ERROR \"@eq 1\" \"phase:2,deny,status:403,id:500077\""
3470+
]
34203471
}
34213472
]

0 commit comments

Comments
 (0)