Skip to content

Commit b638e52

Browse files
victorhorazimmerle
authored andcommitted
Make the boundary check less strict as per RFC2046
1 parent ecad8c6 commit b638e52

File tree

3 files changed

+71
-34
lines changed

3 files changed

+71
-34
lines changed

CHANGES

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
v3.0.4 - YYYY-MMM-DD (to be released)
22
-------------------------------------
33

4-
* Fix buffer size for utf8toUnicode transformation
4+
- Make the boundary check less strict as per RFC2046
5+
[Issue #1943 - @victorhora, @allanbomsft]
6+
- Fix buffer size for utf8toUnicode transformation
57
[Issue #1208 - @katef, @victorhora]
68

79

src/request_body_processor/multipart.cc

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -158,41 +158,27 @@ int Multipart::boundary_characters_valid(const char *boundary) {
158158
}
159159

160160
while ((c = *p) != '\0') {
161-
/* Control characters and space not allowed. */
162-
if (c < 32) {
161+
// Check against allowed list defined in RFC2046 page 22
162+
if (!(
163+
('0' <= c && c <= '9')
164+
|| ('A' <= c && c <= 'Z')
165+
|| ('a' <= c && c <= 'z')
166+
|| (c == ' ' && *(p + 1) != '\0') // space allowed, but not as last character
167+
|| c == '\''
168+
|| c == '('
169+
|| c == ')'
170+
|| c == '+'
171+
|| c == '_'
172+
|| c == ','
173+
|| c == '-'
174+
|| c == '.'
175+
|| c == '/'
176+
|| c == ':'
177+
|| c == '='
178+
|| c == '?'
179+
)) {
163180
return 0;
164181
}
165-
166-
/* Non-ASCII characters not allowed. */
167-
if (c > 126) {
168-
return 0;
169-
}
170-
171-
switch (c) {
172-
/* Special characters not allowed. */
173-
case '(' :
174-
case ')' :
175-
case '<' :
176-
case '>' :
177-
case '@' :
178-
case ',' :
179-
case ';' :
180-
case ':' :
181-
case '\\' :
182-
case '"' :
183-
case '/' :
184-
case '[' :
185-
case ']' :
186-
case '?' :
187-
case '=' :
188-
return 0;
189-
break;
190-
191-
default :
192-
/* Do nothing. */
193-
break;
194-
}
195-
196182
p++;
197183
}
198184

test/test-cases/regression/variable-MULTIPART_STRICT_ERROR.json

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,55 @@
293293
"SecRuleEngine On",
294294
"SecRule MULTIPART_STRICT_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\""
295295
]
296+
},
297+
{
298+
"enabled":1,
299+
"version_min":300000,
300+
"title":"Testing Variables :: MULTIPART_STRICT_ERROR - RFC2046",
301+
"client":{
302+
"ip":"200.249.12.31",
303+
"port":123
304+
},
305+
"server":{
306+
"ip":"200.249.12.31",
307+
"port":80
308+
},
309+
"request":{
310+
"headers":{
311+
"Host":"localhost",
312+
"User-Agent":"curl/7.38.0",
313+
"Accept":"*/*",
314+
"Content-Length":"330",
315+
"Content-Type":"multipart/form-data; boundary=0123456789AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz '()+_,-./:=?",
316+
"Expect":"100-continue"
317+
},
318+
"uri":"/",
319+
"method":"POST",
320+
"body":[
321+
"--0123456789AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz '()+_,-./:=?",
322+
"Content-Disposition: form-data; name=\"name\"",
323+
"",
324+
"1",
325+
"--0123456789AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz '()+_,-./:=?--"
326+
]
327+
},
328+
"response":{
329+
"headers":{
330+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
331+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
332+
"Content-Type":"text/html"
333+
},
334+
"body":[
335+
"no need."
336+
]
337+
},
338+
"expected":{
339+
"debug_log":"Target value: \"0\" \\(Variable: REQBODY_ERROR\\)"
340+
},
341+
"rules":[
342+
"SecRuleEngine On",
343+
"SecRule REQBODY_ERROR \"@contains 0\" \"id:1,phase:3,pass,t:trim\""
344+
]
296345
}
297346
]
298347

0 commit comments

Comments
 (0)