Skip to content

Commit e8bd215

Browse files
zimmerlezimmerle
committed
Having _NAMES, variables proxied
Some variables share content with others; that is the case for ARGS and ARGS_NAMES. Those are different in value, as ARGS_NAMES holds the key name as value. Instead of duplicating the strings for the different collections, this patch unifies the collection in radix, avoiding memory fragmentation. It is currently doing some fragmentation while resolving the variable, but to be mitigated by shared_ptr is VariableValues, a different change. TODO: place others variables such as COOKIE*NAMES to use the same proxy.
1 parent dd458de commit e8bd215

File tree

7 files changed

+359
-16
lines changed

7 files changed

+359
-16
lines changed

‎headers/modsecurity/anchored_set_variable_translation_proxy.h

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* ModSecurity, http://www.modsecurity.org/
3+
* Copyright (c) 2015 - 2021 Trustwave Holdings, Inc. (http://www.trustwave.com/)
4+
*
5+
* You may not use this file except in compliance with
6+
* the License. You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* If any of the files related to licensing are missing or if you have any
11+
* other questions related to licensing please contact Trustwave Holdings, Inc.
12+
* directly using the email address security@modsecurity.org.
13+
*
14+
*/
15+
16+
17+
#ifdef __cplusplus
18+
#include <string>
19+
#include <algorithm>
20+
#include <memory>
21+
#include <functional>
22+
#include <iostream>
23+
#endif
24+
25+
#include "modsecurity/variable_value.h"
26+
#include "modsecurity/anchored_set_variable.h"
27+
28+
29+
#ifndef HEADERS_MODSECURITY_ANCHORED_SET_VARIABLE_TRANSLATION_PROXY_H_
30+
#define HEADERS_MODSECURITY_ANCHORED_SET_VARIABLE_TRANSLATION_PROXY_H_
31+
32+
#ifdef __cplusplus
33+
34+
namespace modsecurity {
35+
36+
37+
class AnchoredSetVariableTranslationProxy {
38+
public:
39+
AnchoredSetVariableTranslationProxy(
40+
const std::string &name,
41+
AnchoredSetVariable *fount)
42+
: m_name(name),
43+
m_fount(fount)
44+
{
45+
m_translate = [](std::string *name, std::vector<const VariableValue *> *l) {
46+
for (int i = 0; i < l->size(); ++i) {
47+
VariableValue *newVariableValue = new VariableValue(name, &l->at(i)->getKey());
48+
const VariableValue *oldVariableValue = l->at(i);
49+
l->at(i) = newVariableValue;
50+
for (auto &oldOrigin : oldVariableValue->getOrigin()) {
51+
std::unique_ptr<VariableOrigin> newOrigin(new VariableOrigin);
52+
newOrigin->m_length = oldVariableValue->getKey().size();
53+
newOrigin->m_offset = oldOrigin->m_offset - oldVariableValue->getKey().size() - 1;
54+
newVariableValue->addOrigin(std::move(newOrigin));
55+
}
56+
delete oldVariableValue;
57+
}
58+
};
59+
}
60+
61+
virtual ~AnchoredSetVariableTranslationProxy()
62+
{ }
63+
64+
void resolve(std::vector<const VariableValue *> *l) {
65+
m_fount->resolve(l);
66+
m_translate(&m_name, l);
67+
}
68+
69+
void resolve(std::vector<const VariableValue *> *l,
70+
variables::KeyExclusions &ke) {
71+
m_fount->resolve(l, ke);
72+
m_translate(&m_name, l);
73+
}
74+
75+
void resolve(const std::string &key,
76+
std::vector<const VariableValue *> *l) {
77+
m_fount->resolve(key, l);
78+
m_translate(&m_name, l);
79+
};
80+
81+
void resolveRegularExpression(Utils::Regex *r,
82+
std::vector<const VariableValue *> *l) {
83+
m_fount->resolveRegularExpression(r, l);
84+
m_translate(&m_name, l);
85+
};
86+
87+
void resolveRegularExpression(Utils::Regex *r,
88+
std::vector<const VariableValue *> *l,
89+
variables::KeyExclusions &ke) {
90+
m_fount->resolveRegularExpression(r, l, ke);
91+
m_translate(&m_name, l);
92+
};
93+
94+
std::unique_ptr<std::string> resolveFirst(const std::string &key) {
95+
std::vector<const VariableValue *> l;
96+
resolve(&l);
97+
98+
if (l.empty()) {
99+
return nullptr;
100+
}
101+
102+
std::unique_ptr<std::string> ret(new std::string(""));
103+
104+
ret->assign(l.at(0)->getValue());
105+
106+
while (!l.empty()) {
107+
auto &a = l.back();
108+
l.pop_back();
109+
delete a;
110+
}
111+
112+
return ret;
113+
}
114+
115+
std::string m_name;
116+
private:
117+
AnchoredSetVariable *m_fount;
118+
std::function<void(std::string *name, std::vector<const VariableValue *> *l)> m_translate;
119+
};
120+
121+
} // namespace modsecurity
122+
123+
#endif
124+
125+
126+
#endif // HEADERS_MODSECURITY_ANCHORED_SET_VARIABLE_TRANSLATION_PROXY_H_

‎headers/modsecurity/transaction.h

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ typedef struct Rules_t RulesSet;
4848
#include "modsecurity/variable_value.h"
4949
#include "modsecurity/collection/collection.h"
5050
#include "modsecurity/variable_origin.h"
51+
#include "modsecurity/anchored_set_variable_translation_proxy.h"
5152

5253

5354
#ifndef NO_LOGS
@@ -121,10 +122,7 @@ class Operator;
121122
class TransactionAnchoredVariables {
122123
public:
123124
explicit TransactionAnchoredVariables(Transaction *t)
124-
: m_variableArgsNames(t, "ARGS_NAMES"),
125-
m_variableArgsGetNames(t, "ARGS_GET_NAMES"),
126-
m_variableArgsPostNames(t, "ARGS_POST_NAMES"),
127-
m_variableRequestHeadersNames(t, "REQUEST_HEADERS_NAMES"),
125+
: m_variableRequestHeadersNames(t, "REQUEST_HEADERS_NAMES"),
128126
m_variableResponseContentType(t, "RESPONSE_CONTENT_TYPE"),
129127
m_variableResponseHeadersNames(t, "RESPONSE_HEADERS_NAMES"),
130128
m_variableARGScombinedSize(t, "ARGS_COMBINED_SIZE"),
@@ -202,12 +200,12 @@ class TransactionAnchoredVariables {
202200
m_variableGeo(t, "GEO"),
203201
m_variableRequestCookiesNames(t, "REQUEST_COOKIES_NAMES"),
204202
m_variableFilesTmpNames(t, "FILES_TMPNAMES"),
205-
m_variableOffset(0)
203+
m_variableOffset(0),
204+
m_variableArgsNames("ARGS_NAMES", &m_variableArgs),
205+
m_variableArgsGetNames("ARGS_GET_NAMES", &m_variableArgsGet),
206+
m_variableArgsPostNames("ARGS_POST_NAMES", &m_variableArgsPost)
206207
{ }
207208

208-
AnchoredSetVariable m_variableArgsNames;
209-
AnchoredSetVariable m_variableArgsGetNames;
210-
AnchoredSetVariable m_variableArgsPostNames;
211209
AnchoredSetVariable m_variableRequestHeadersNames;
212210
AnchoredVariable m_variableResponseContentType;
213211
AnchoredSetVariable m_variableResponseHeadersNames;
@@ -285,6 +283,10 @@ class TransactionAnchoredVariables {
285283
AnchoredSetVariable m_variableFilesTmpNames;
286284

287285
int m_variableOffset;
286+
287+
AnchoredSetVariableTranslationProxy m_variableArgsNames;
288+
AnchoredSetVariableTranslationProxy m_variableArgsGetNames;
289+
AnchoredSetVariableTranslationProxy m_variableArgsPostNames;
288290
};
289291

290292
class TransactionSecMarkerManagement {

‎src/Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ MAINTAINERCLEANFILES = \
3535

3636

3737
pkginclude_HEADERS = \
38+
../headers/modsecurity/anchored_set_variable_translation_proxy.h \
3839
../headers/modsecurity/anchored_set_variable.h \
3940
../headers/modsecurity/anchored_variable.h \
4041
../headers/modsecurity/audit_log.h \

‎src/transaction.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,17 +393,13 @@ bool Transaction::addArgument(const std::string& orig, const std::string& key,
393393
return false;
394394
}
395395

396-
size_t k_offset = offset;
397396
offset = offset + key.size() + 1;
398397
m_variableArgs.set(key, value, offset);
399-
m_variableArgsNames.set(key, key, k_offset);
400398

401399
if (orig == "GET") {
402400
m_variableArgsGet.set(key, value, offset);
403-
m_variableArgsGetNames.set(key, key, k_offset);
404401
} else if (orig == "POST") {
405402
m_variableArgsPost.set(key, value, offset);
406-
m_variableArgsPostNames.set(key, key, k_offset);
407403
}
408404

409405
m_ARGScombinedSizeDouble = m_ARGScombinedSizeDouble + \

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

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -882,11 +882,11 @@
882882
]
883883
},
884884
"expected":{
885-
"error_log":"o0,1v228,1t:lowercase"
885+
"error_log":"o0,1v228,1t:lowercase,t:trim"
886886
},
887887
"rules":[
888888
"SecRequestBodyAccess On",
889-
"SecRule REQUEST_COOKIES \"b\" \"id:1,phase:2,pass,t:lowercase,msg:'ops'\""
889+
"SecRule REQUEST_COOKIES \"b\" \"id:1,phase:2,pass,t:lowercase,t:trim,msg:'ops'\""
890890
]
891891
},
892892
{
@@ -1951,5 +1951,67 @@
19511951
"SecUploadDir /tmp",
19521952
"SecRule MULTIPART_NAME \"fiasdfasdfledata\" \"id:1,phase:3,pass,t:trim,msg:'s'\""
19531953
]
1954+
},
1955+
{
1956+
"enabled":1,
1957+
"version_min":300000,
1958+
"title":"Variable offset - ARGS n",
1959+
"request":{
1960+
"headers":{
1961+
"Host":"localhost",
1962+
"Content-Length": "27",
1963+
"Content-Type": "application/x-www-form-urlencoded"
1964+
},
1965+
"uri":"/index.html?param01=5555&bbbbbbbmy_id=6",
1966+
"method":"GET"
1967+
},
1968+
"response":{
1969+
"headers":{
1970+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
1971+
"Content-Type":"text/html"
1972+
},
1973+
"body":[
1974+
"no need."
1975+
]
1976+
},
1977+
"expected":{
1978+
"http_code": 403,
1979+
"error_log":"o0,1v42,1"
1980+
},
1981+
"rules":[
1982+
"SecRuleEngine On",
1983+
"SecRule ARGS \"@contains 6\" \"id:1,phase:2,deny,status:403,log\""
1984+
]
1985+
},
1986+
{
1987+
"enabled":1,
1988+
"version_min":300000,
1989+
"title":"Variable offset - ARGS_NAMES n",
1990+
"request":{
1991+
"headers":{
1992+
"Host":"localhost",
1993+
"Content-Length": "27",
1994+
"Content-Type": "application/x-www-form-urlencoded"
1995+
},
1996+
"uri":"/index.html?param01=5555&bbbbbbbmy_id=6",
1997+
"method":"GET"
1998+
},
1999+
"response":{
2000+
"headers":{
2001+
"Date":"Mon, 13 Jul 2015 20:02:41 GMT",
2002+
"Content-Type":"text/html"
2003+
},
2004+
"body":[
2005+
"no need."
2006+
]
2007+
},
2008+
"expected":{
2009+
"http_code": 403,
2010+
"error_log":"o7,5v29,12"
2011+
},
2012+
"rules":[
2013+
"SecRuleEngine On",
2014+
"SecRule ARGS_NAMES \"@contains my_id\" \"id:1,phase:2,deny,status:403,log\""
2015+
]
19542016
}
19552017
]

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,58 @@
164164
"SecRuleEngine On",
165165
"SecRule ARGS_NAMES \"@contains test \" \"id:1,phase:3,pass,t:trim\""
166166
]
167+
},
168+
{
169+
"enabled":1,
170+
"version_min":300000,
171+
"title":"Testing Variables :: ARGS_POST_NAMES (3/x)",
172+
"client":{
173+
"ip":"200.249.12.31",
174+
"port":123
175+
},
176+
"server":{
177+
"ip":"200.249.12.31",
178+
"port":80
179+
},
180+
"request":{
181+
"headers":{
182+
"Host":"localhost",
183+
"User-Agent":"curl/7.38.0",
184+
"Accept":"*/*",
185+
"Content-Length":"330",
186+
"Content-Type":"multipart/form-data; boundary=0000",
187+
"Expect":"100-continue"
188+
},
189+
"uri":"/",
190+
"method":"POST",
191+
"body":[
192+
"--0000\r",
193+
"Content-Disposition: form-data; name=\"name1\"\r",
194+
"\r",
195+
"content1\r",
196+
"--0000\r",
197+
"Content-Disposition: form-data; name=\"name2\"\r",
198+
"\r",
199+
"content2\r",
200+
"--0000--\r"
201+
]
202+
},
203+
"response":{
204+
"headers":{
205+
"Content-Type":"text/html"
206+
},
207+
"body":[
208+
"no need."
209+
]
210+
},
211+
"expected":{
212+
"debug_log":"Target value: \"name1\" \\(Variable: ARGS_NAMES\\)"
213+
},
214+
"rules":[
215+
"SecRuleEngine On",
216+
"SecRequestBodyAccess On",
217+
"SecRule ARGS_NAMES \"@contains test \" \"id:1,phase:3,pass,t:trim\""
218+
]
167219
}
168220
]
169221

0 commit comments

Comments
 (0)