Skip to content

Commit c7ad6c7

Browse files
airweenairween
committed
Replace Cookie parsing method
1 parent 199a9db commit c7ad6c7

File tree

3 files changed

+170
-17
lines changed

3 files changed

+170
-17
lines changed

‎src/transaction.cc‎

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -548,20 +548,53 @@ int Transaction::addRequestHeader(const std::string& key,
548548

549549
if (keyl == "cookie") {
550550
size_t localOffset = m_variableOffset;
551+
size_t pos;
551552
std::vector<std::string> cookies = utils::string::ssplit(value, ';');
552553
for (const std::string &c : cookies) {
553-
std::vector<std::string> s = utils::string::split(c,
554-
'=');
555-
if (s.size() > 1) {
556-
if (s[0].at(0) == ' ') {
557-
s[0].erase(0, 1);
558-
}
559-
m_variableRequestCookiesNames.set(s[0],
560-
s[0], localOffset);
554+
// skip empty substring, eg "Cookie: ;;foo=bar"
555+
if (c.empty() == true) {
556+
localOffset++; // add length of ';'
557+
continue;
558+
}
559+
560+
// find the first '='
561+
pos = c.find_first_of("=", 0);
562+
std::string ckey = "";
563+
std::string cval = "";
561564

562-
localOffset = localOffset + s[0].size() + 1;
563-
m_variableRequestCookies.set(s[0], s[1], localOffset);
564-
localOffset = localOffset + s[1].size() + 2;
565+
// if the cookie doesn't contains '=', its just a key
566+
if (pos == std::string::npos) {
567+
ckey = c;
568+
}
569+
// else split to two substrings by first =
570+
else {
571+
ckey = c.substr(0, pos);
572+
// value will contains the next '=' chars if exists
573+
// eg. foo=bar=baz -> key: foo, value: bar=baz
574+
cval = c.substr(pos+1);
575+
}
576+
577+
// ltrim the key - following the modsec v2 way
578+
while (ckey.empty() == false && ckey.at(0) == ' ') {
579+
ckey.erase(0, 1);
580+
localOffset++;
581+
}
582+
583+
// if the key is empty (eg: "Cookie: =bar;") skip it
584+
if (ckey.empty() == true) {
585+
localOffset = localOffset + c.length() + 1;
586+
continue;
587+
}
588+
else {
589+
// handle cookie only if the key is not empty
590+
// set cookie name
591+
m_variableRequestCookiesNames.set(ckey,
592+
ckey, localOffset);
593+
localOffset = localOffset + ckey.size() + 1;
594+
// set cookie value
595+
m_variableRequestCookies.set(ckey, cval,
596+
localOffset);
597+
localOffset = localOffset + cval.size() + 1;
565598
}
566599
}
567600
}

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

Lines changed: 83 additions & 3 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 :: REQUEST_COOKIES (1/3)",
5+
"title":"Testing Variables :: REQUEST_COOKIES (1/5)",
66
"client":{
77
"ip":"200.249.12.31",
88
"port":123
@@ -42,7 +42,7 @@
4242
{
4343
"enabled":1,
4444
"version_min":300000,
45-
"title":"Testing Variables :: REQUEST_COOKIES (2/3)",
45+
"title":"Testing Variables :: REQUEST_COOKIES (2/5)",
4646
"client":{
4747
"ip":"200.249.12.31",
4848
"port":123
@@ -82,7 +82,7 @@
8282
{
8383
"enabled":1,
8484
"version_min":300000,
85-
"title":"Testing Variables :: REQUEST_COOKIES (3/3)",
85+
"title":"Testing Variables :: REQUEST_COOKIES (3/5)",
8686
"client":{
8787
"ip":"200.249.12.31",
8888
"port":123
@@ -118,6 +118,86 @@
118118
"SecRuleEngine On",
119119
"SecRule REQUEST_COOKIES \"@contains test \" \"id:1,pass,t:trim\""
120120
]
121+
},
122+
{
123+
"enabled":1,
124+
"version_min":300000,
125+
"title":"Testing Variables :: REQUEST_COOKIES (4/5)",
126+
"client":{
127+
"ip":"200.249.12.31",
128+
"port":123
129+
},
130+
"server":{
131+
"ip":"200.249.12.31",
132+
"port":80
133+
},
134+
"request":{
135+
"headers":{
136+
"Host":"localhost",
137+
"User-Agent":"curl/7.38.0",
138+
"Accept":"*/*",
139+
"Cookie":"USER_TOKEN=Yes; a=z; t=b; foo= bar"
140+
},
141+
"uri":"/?key=value&key=other_value",
142+
"method":"GET"
143+
},
144+
"response":{
145+
"headers":{
146+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
147+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
148+
"Content-Type":"text/html"
149+
},
150+
"body":[
151+
"no need."
152+
]
153+
},
154+
"expected":{
155+
"debug_log":"Target value: \"bar\""
156+
},
157+
"rules":[
158+
"SecRuleEngine On",
159+
"SecRule REQUEST_COOKIES \"@contains test \" \"id:1,pass,t:trim\""
160+
]
161+
},
162+
{
163+
"enabled":1,
164+
"version_min":300000,
165+
"title":"Testing Variables :: REQUEST_COOKIES (5/5)",
166+
"client":{
167+
"ip":"200.249.12.31",
168+
"port":123
169+
},
170+
"server":{
171+
"ip":"200.249.12.31",
172+
"port":80
173+
},
174+
"request":{
175+
"headers":{
176+
"Host":"localhost",
177+
"User-Agent":"curl/7.38.0",
178+
"Accept":"*/*",
179+
"Cookie":"USER_TOKEN=Yes; a=z; t=b; foo= bar; = ; = = ; baz=value1=insert here something"
180+
},
181+
"uri":"/?key=value&key=other_value",
182+
"method":"GET"
183+
},
184+
"response":{
185+
"headers":{
186+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
187+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
188+
"Content-Type":"text/html"
189+
},
190+
"body":[
191+
"no need."
192+
]
193+
},
194+
"expected":{
195+
"debug_log":"Target value: \"value1=insert here something\""
196+
},
197+
"rules":[
198+
"SecRuleEngine On",
199+
"SecRule REQUEST_COOKIES \"@contains test \" \"id:1,pass,t:trim\""
200+
]
121201
}
122202
]
123203

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

Lines changed: 43 additions & 3 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 :: REQUEST_COOKIES_NAMES (1/3)",
5+
"title":"Testing Variables :: REQUEST_COOKIES_NAMES (1/4)",
66
"client":{
77
"ip":"200.249.12.31",
88
"port":123
@@ -42,7 +42,7 @@
4242
{
4343
"enabled":1,
4444
"version_min":300000,
45-
"title":"Testing Variables :: REQUEST_COOKIES_NAMES (2/3)",
45+
"title":"Testing Variables :: REQUEST_COOKIES_NAMES (2/4)",
4646
"client":{
4747
"ip":"200.249.12.31",
4848
"port":123
@@ -82,7 +82,7 @@
8282
{
8383
"enabled":1,
8484
"version_min":300000,
85-
"title":"Testing Variables :: REQUEST_COOKIES_NAMES (3/3)",
85+
"title":"Testing Variables :: REQUEST_COOKIES_NAMES (3/4)",
8686
"client":{
8787
"ip":"200.249.12.31",
8888
"port":123
@@ -118,6 +118,46 @@
118118
"SecRuleEngine On",
119119
"SecRule REQUEST_COOKIES_NAMES \"@contains test \" \"id:1,pass,t:trim\""
120120
]
121+
},
122+
{
123+
"enabled":1,
124+
"version_min":300000,
125+
"title":"Testing Variables :: REQUEST_COOKIES_NAMES (4/4)",
126+
"client":{
127+
"ip":"200.249.12.31",
128+
"port":123
129+
},
130+
"server":{
131+
"ip":"200.249.12.31",
132+
"port":80
133+
},
134+
"request":{
135+
"headers":{
136+
"Host":"localhost",
137+
"User-Agent":"curl/7.38.0",
138+
"Accept":"*/*",
139+
"Cookie":"USER_TOKEN=Yes; a=z; t=b; foobar"
140+
},
141+
"uri":"/?key=value&key=other_value",
142+
"method":"GET"
143+
},
144+
"response":{
145+
"headers":{
146+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
147+
"Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT",
148+
"Content-Type":"text/html"
149+
},
150+
"body":[
151+
"no need."
152+
]
153+
},
154+
"expected":{
155+
"debug_log":"Target value: \"foobar\""
156+
},
157+
"rules":[
158+
"SecRuleEngine On",
159+
"SecRule REQUEST_COOKIES_NAMES \"@contains foobar \" \"id:1,pass,t:trim\""
160+
]
121161
}
122162
]
123163

0 commit comments

Comments
 (0)