Skip to content

Commit cd5fba8

Browse files
martinhsvzimmerle
martinhsv
authored and
zimmerle
committed
Handle URI received with uri-fragment
1 parent faad65d commit cd5fba8

File tree

7 files changed

+390
-19
lines changed

7 files changed

+390
-19
lines changed

‎CHANGES‎

Lines changed: 2 additions & 0 deletions
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+
- Handle URI received with uri-fragment
5+
[@martinhsv]
46
- Having ARGS_NAMES, variables proxied
57
[@zimmerle, @martinhsv, @KaNikita]
68
- Use explicit path for cross-compile environments.

‎src/transaction.cc‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -445,11 +445,21 @@ int Transaction::processURI(const char *uri, const char *method,
445445
m_httpVersion = http_version;
446446
m_uri = uri;
447447
std::string uri_s(uri);
448-
m_uri_decoded = utils::uri_decode(uri);
449448

450-
size_t pos = m_uri_decoded.find("?");
451-
size_t pos_raw = uri_s.find("?");
452-
size_t var_size = pos_raw;
449+
// any uri-fragment that was received should only be retained in
450+
// - m_uri
451+
// - m_variableRequestURIRaw
452+
// - m_variableRequestLine
453+
size_t pos_raw_fragment = uri_s.find("#");
454+
if (pos_raw_fragment != std::string::npos) {
455+
uri_s = uri_s.substr(0, pos_raw_fragment);
456+
}
457+
458+
size_t pos_raw_query = uri_s.find("?");
459+
460+
m_uri_decoded = utils::uri_decode(uri_s);
461+
462+
size_t var_size = pos_raw_query;
453463

454464
m_variableRequestMethod.set(method, 0);
455465

@@ -462,28 +472,28 @@ int Transaction::processURI(const char *uri, const char *method,
462472
m_variableOffset + requestLine.size() + 1);
463473

464474

465-
466-
if (pos != std::string::npos) {
475+
size_t pos_query = m_uri_decoded.find("?");
476+
if (pos_query != std::string::npos) {
467477
m_uri_no_query_string_decoded = std::unique_ptr<std::string>(
468-
new std::string(m_uri_decoded, 0, pos));
478+
new std::string(m_uri_decoded, 0, pos_query));
469479
} else {
470480
m_uri_no_query_string_decoded = std::unique_ptr<std::string>(
471481
new std::string(m_uri_decoded));
472482
}
473483

474484

475-
if (pos_raw != std::string::npos) {
476-
std::string qry = std::string(uri_s, pos_raw + 1,
477-
uri_s.length() - (pos_raw + 1));
478-
m_variableQueryString.set(qry, pos_raw + 1
485+
if (pos_raw_query != std::string::npos) {
486+
std::string qry = std::string(uri_s, pos_raw_query + 1,
487+
uri_s.length() - (pos_raw_query + 1));
488+
m_variableQueryString.set(qry, pos_raw_query + 1
479489
+ std::string(method).size() + 1);
480490
}
481491

482492
std::string path_info;
483-
if (pos == std::string::npos) {
493+
if (pos_query == std::string::npos) {
484494
path_info = std::string(m_uri_decoded, 0);
485495
} else {
486-
path_info = std::string(m_uri_decoded, 0, pos);
496+
path_info = std::string(m_uri_decoded, 0, pos_query);
487497
}
488498
if (var_size == std::string::npos) {
489499
var_size = uri_s.size();
@@ -495,7 +505,6 @@ int Transaction::processURI(const char *uri, const char *method,
495505
strlen(method) + 1, var_size);
496506

497507

498-
499508
size_t offset = path_info.find_last_of("/\\");
500509
if (offset != std::string::npos && path_info.length() > offset + 1) {
501510
std::string basename = std::string(path_info, offset + 1,

‎test/test-cases/regression/variable-ARGS_GET.json‎

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
{
33
"enabled":1,
44
"version_min":300000,
5-
"title":"Testing Variables :: ARGS_GET (1/5)",
5+
"title":"Testing Variables :: ARGS_GET (1/6)",
66
"client":{
77
"ip":"200.249.12.31",
88
"port":123
@@ -41,7 +41,7 @@
4141
{
4242
"enabled":1,
4343
"version_min":300000,
44-
"title":"Testing Variables :: ARGS_GET (2/5)",
44+
"title":"Testing Variables :: ARGS_GET (2/6)",
4545
"client":{
4646
"ip":"200.249.12.31",
4747
"port":123
@@ -80,7 +80,7 @@
8080
{
8181
"enabled":1,
8282
"version_min":300000,
83-
"title":"Testing Variables :: ARGS_GET (3/5)",
83+
"title":"Testing Variables :: ARGS_GET (3/6)",
8484
"client":{
8585
"ip":"200.249.12.31",
8686
"port":123
@@ -119,7 +119,7 @@
119119
{
120120
"enabled":1,
121121
"version_min":300000,
122-
"title":"Testing Variables :: ARGS_GET (4/5)",
122+
"title":"Testing Variables :: ARGS_GET (4/6)",
123123
"client":{
124124
"ip":"200.249.12.31",
125125
"port":123
@@ -158,7 +158,7 @@
158158
{
159159
"enabled":1,
160160
"version_min":300000,
161-
"title":"Testing Variables :: ARGS_GET (5/5)",
161+
"title":"Testing Variables :: ARGS_GET (5/6)",
162162
"client":{
163163
"ip":"200.249.12.31",
164164
"port":123
@@ -193,6 +193,46 @@
193193
"SecRuleEngine On",
194194
"SecRule ARGS_GET \"@rx ^othervalue$ \" \"id:1,pass,t:none\""
195195
]
196+
},
197+
{
198+
"enabled":1,
199+
"version_min":300000,
200+
"title":"Testing Variables :: ARGS_GET (6/6)",
201+
"client":{
202+
"ip":"200.249.12.31",
203+
"port":123
204+
},
205+
"server":{
206+
"ip":"200.249.12.31",
207+
"port":80
208+
},
209+
"request":{
210+
"headers":{
211+
"Host":"localhost",
212+
"User-Agent":"curl/7.38.0",
213+
"Accept":"*/*"
214+
},
215+
"uri":"/?key=value&second_key=other_value#urifrag",
216+
"method":"GET",
217+
"http_version":1.1
218+
},
219+
"response":{
220+
"headers":{
221+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
222+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
223+
"Content-Type":"text/html"
224+
},
225+
"body":[
226+
"no need."
227+
]
228+
},
229+
"expected":{
230+
"http_code": 403
231+
},
232+
"rules":[
233+
"SecRuleEngine On",
234+
"SecRule ARGS_GET \"@streq other_value\" \"id:1,phase:1,deny,status:403\""
235+
]
196236
}
197237
]
198238

‎test/test-cases/regression/variable-QUERY_STRING.json‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,5 +39,85 @@
3939
"SecRuleEngine On",
4040
"SecRule QUERY_STRING \"@contains test \" \"id:1,phase:3,pass,t:trim\""
4141
]
42+
},
43+
{
44+
"enabled":1,
45+
"version_min":300000,
46+
"title":"Testing Variables :: QUERY_STRING (URI contains fragment)",
47+
"client":{
48+
"ip":"200.249.12.31",
49+
"port":123
50+
},
51+
"server":{
52+
"ip":"200.249.12.31",
53+
"port":80
54+
},
55+
"request":{
56+
"headers":{
57+
"Host":"localhost",
58+
"User-Agent":"curl/7.38.0",
59+
"Accept":"*/*"
60+
},
61+
"uri":"/?key=value&key=other_value#urifrag",
62+
"method":"GET",
63+
"http_version":1.1
64+
},
65+
"response":{
66+
"headers":{
67+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
68+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
69+
"Content-Type":"text/html"
70+
},
71+
"body":[
72+
"no need."
73+
]
74+
},
75+
"expected":{
76+
"http_code": 403
77+
},
78+
"rules":[
79+
"SecRuleEngine On",
80+
"SecRule QUERY_STRING \"!@contains urifrag\" \"id:1,phase:1,deny,status:403\""
81+
]
82+
},
83+
{
84+
"enabled":1,
85+
"version_min":300000,
86+
"title":"Testing Variables :: QUERY_STRING (URI contains fragment)",
87+
"client":{
88+
"ip":"200.249.12.31",
89+
"port":123
90+
},
91+
"server":{
92+
"ip":"200.249.12.31",
93+
"port":80
94+
},
95+
"request":{
96+
"headers":{
97+
"Host":"localhost",
98+
"User-Agent":"curl/7.38.0",
99+
"Accept":"*/*"
100+
},
101+
"uri":"/one/two/testpost.php#urifrag",
102+
"method":"GET",
103+
"http_version":1.1
104+
},
105+
"response":{
106+
"headers":{
107+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
108+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
109+
"Content-Type":"text/html"
110+
},
111+
"body":[
112+
"no need."
113+
]
114+
},
115+
"expected":{
116+
"http_code": 403
117+
},
118+
"rules":[
119+
"SecRuleEngine On",
120+
"SecRule QUERY_STRING \"@eq 0\" \"id:1,phase:1,t:length,deny,status:403\""
121+
]
42122
}
43123
]

‎test/test-cases/regression/variable-REQUEST_LINE.json‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,86 @@
3838
"SecRuleEngine On",
3939
"SecRule REQUEST_LINE \"@contains test \" \"id:1,pass,t:trim\""
4040
]
41+
},
42+
{
43+
"enabled":1,
44+
"version_min":300000,
45+
"title":"Testing Variables :: REQUEST_LINE (with URI fragment)",
46+
"client":{
47+
"ip":"200.249.12.31",
48+
"port":123
49+
},
50+
"server":{
51+
"ip":"200.249.12.31",
52+
"port":80
53+
},
54+
"request":{
55+
"headers":{
56+
"Host":"localhost",
57+
"User-Agent":"curl/7.38.0",
58+
"Accept":"*/*"
59+
},
60+
"uri":"/?key=value&key=other_value#urifrag",
61+
"method":"GET",
62+
"http_version":1.1
63+
},
64+
"response":{
65+
"headers":{
66+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
67+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
68+
"Content-Type":"text/html"
69+
},
70+
"body":[
71+
"no need."
72+
]
73+
},
74+
"expected":{
75+
"http_code": 403
76+
},
77+
"rules":[
78+
"SecRuleEngine On",
79+
"SecRule REQUEST_LINE \"@contains urifrag\" \"id:1,phase:1,deny,status:403\""
80+
]
81+
},
82+
{
83+
"enabled":1,
84+
"version_min":300000,
85+
"title":"Testing Variables :: REQUEST_URI_RAW (with URI fragment)",
86+
"client":{
87+
"ip":"200.249.12.31",
88+
"port":123
89+
},
90+
"server":{
91+
"ip":"200.249.12.31",
92+
"port":80
93+
},
94+
"request":{
95+
"headers":{
96+
"Host":"localhost",
97+
"User-Agent":"curl/7.38.0",
98+
"Accept":"*/*"
99+
},
100+
"uri":"/one/two/testpost.php#urifrag",
101+
"method":"GET",
102+
"http_version":1.1
103+
},
104+
"response":{
105+
"headers":{
106+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
107+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
108+
"Content-Type":"text/html"
109+
},
110+
"body":[
111+
"no need."
112+
]
113+
},
114+
"expected":{
115+
"http_code": 403
116+
},
117+
"rules":[
118+
"SecRuleEngine On",
119+
"SecRule REQUEST_URI_RAW \"@contains urifrag\" \"id:1,phase:1,deny,status:403\""
120+
]
41121
}
42122
]
43123

0 commit comments

Comments
 (0)