Skip to content

Commit 4951702

Browse files
airweenairween
authoredAug 28, 2024
Merge pull request #3233 from eduar-hte/remove-copies-pm-operator
Removed multiple heap-allocated copies in Pm::init & parse_pm_content
2 parents 97c8766 + 3e9d810 commit 4951702

File tree

6 files changed

+75
-189
lines changed

6 files changed

+75
-189
lines changed
 

‎src/Makefile.am

-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ OPERATORS = \
219219
operators/no_match.cc \
220220
operators/operator.cc \
221221
operators/pm.cc \
222-
operators/pm_f.cc \
223222
operators/pm_from_file.cc \
224223
operators/rbl.cc \
225224
operators/rsub.cc \

‎src/operators/pm.cc

+73-30
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,80 @@
1515

1616
#include "src/operators/pm.h"
1717

18-
#include <string.h>
19-
20-
#include <string>
2118
#include <algorithm>
2219
#include <iterator>
2320
#include <sstream>
24-
#include <vector>
25-
#include <list>
26-
#include <memory>
2721

28-
#include "src/operators/operator.h"
2922
#include "src/utils/acmp.h"
3023
#include "src/utils/string.h"
3124

25+
26+
static inline std::string parse_pm_content(const std::string &op_parm) {
27+
auto offset = op_parm.find_first_not_of(" \t");
28+
if (offset == std::string::npos) {
29+
return op_parm;
30+
}
31+
32+
auto size = op_parm.size() - offset;
33+
if (size >= 2 &&
34+
op_parm.at(offset) == '\"' && op_parm.back() == '\"') {
35+
offset++;
36+
size -= 2;
37+
}
38+
39+
if (size == 0) {
40+
return op_parm;
41+
}
42+
43+
std::string parsed_parm(op_parm.c_str() + offset, size);
44+
45+
unsigned char bin_offset = 0;
46+
unsigned char bin_parm[3] = { 0 };
47+
bool bin = false, esc = false;
48+
49+
char *d = parsed_parm.data();
50+
for(const char *s = d, *e = d + size; s != e; ++s) {
51+
if (*s == '|') {
52+
bin = !bin;
53+
} else if(!esc && *s == '\\') {
54+
esc = true;
55+
} else {
56+
if (bin) {
57+
if (VALID_HEX(*s)) {
58+
bin_parm[bin_offset] = (char)*s;
59+
bin_offset++;
60+
if (bin_offset == 2) {
61+
unsigned char c = strtol((char *)bin_parm, (char **) nullptr, 16) & 0xFF;
62+
bin_offset = 0;
63+
*d++ = c;
64+
}
65+
} else {
66+
// Invalid hex character
67+
return op_parm;
68+
}
69+
} else if (esc) {
70+
if (*s == ':' ||
71+
*s == ';' ||
72+
*s == '\\' ||
73+
*s == '\"')
74+
{
75+
*d++ = *s;
76+
} else {
77+
// Unsupported escape sequence
78+
return op_parm;
79+
}
80+
esc = false;
81+
} else {
82+
*d++ = *s;
83+
}
84+
}
85+
}
86+
87+
parsed_parm.resize(d - parsed_parm.c_str());
88+
return parsed_parm;
89+
}
90+
91+
3292
namespace modsecurity {
3393
namespace operators {
3494

@@ -105,36 +165,19 @@ bool Pm::evaluate(Transaction *transaction, RuleWithActions *rule,
105165

106166

107167
bool Pm::init(const std::string &file, std::string *error) {
108-
std::vector<std::string> vec;
109-
std::istringstream *iss;
110-
const char *err = NULL;
111-
112-
char *content = parse_pm_content(m_param.c_str(), m_param.length(), &err);
113-
if (content == NULL) {
114-
iss = new std::istringstream(m_param);
115-
} else {
116-
iss = new std::istringstream(content);
117-
}
168+
const auto op_parm = parse_pm_content(m_param);
118169

119-
std::copy(std::istream_iterator<std::string>(*iss),
170+
std::istringstream iss{op_parm};
171+
std::for_each(std::istream_iterator<std::string>(iss),
120172
std::istream_iterator<std::string>(),
121-
back_inserter(vec));
122-
123-
for (auto &a : vec) {
124-
acmp_add_pattern(m_p, a.c_str(), NULL, NULL, a.length());
125-
}
173+
[this](const auto &a) {
174+
acmp_add_pattern(m_p, a.c_str(), NULL, NULL, a.length());
175+
});
126176

127177
while (m_p->is_failtree_done == 0) {
128178
acmp_prepare(m_p);
129179
}
130180

131-
if (content) {
132-
free(content);
133-
content = NULL;
134-
}
135-
136-
delete iss;
137-
138181
return true;
139182
}
140183

‎src/operators/pm.h

-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#define SRC_OPERATORS_PM_H_
1818

1919
#include <string>
20-
#include <list>
2120
#include <memory>
2221
#include <utility>
2322
#include <mutex>

‎src/operators/pm_f.cc

-27
This file was deleted.

‎src/utils/acmp.cc

+1-128
Original file line numberDiff line numberDiff line change
@@ -33,135 +33,8 @@
3333
* that should be mitigated. This ACMP parser should be re-written to
3434
* consume less memory.
3535
*/
36-
extern "C" {
37-
38-
char *parse_pm_content(const char *op_parm, unsigned short int op_len, const char **error_msg) {
39-
char *parm = NULL;
40-
char *content;
41-
unsigned short int offset = 0;
42-
// char converted = 0;
43-
int i, x;
44-
unsigned char bin = 0, esc = 0, bin_offset = 0;
45-
unsigned char c = 0;
46-
unsigned char bin_parm[3] = { 0 };
47-
char *processed = NULL;
48-
49-
content = strdup(op_parm);
50-
51-
if (content == NULL) {
52-
*error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
53-
return NULL;
54-
}
55-
56-
while (offset < op_len && (content[offset] == ' ' || content[offset] == '\t')) {
57-
offset++;
58-
};
59-
60-
op_len = strlen(content);
61-
62-
if (content[offset] == '\"' && content[op_len-1] == '\"') {
63-
parm = strdup(content + offset + 1);
64-
if (parm == NULL) {
65-
*error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
66-
free(content);
67-
content = NULL;
68-
return NULL;
69-
}
70-
parm[op_len - offset - 2] = '\0';
71-
} else {
72-
parm = strdup(content + offset);
73-
if (parm == NULL) {
74-
free(content);
75-
content = NULL;
76-
*error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
77-
return NULL;
78-
}
79-
}
80-
81-
free(content);
82-
content = NULL;
8336

84-
op_len = strlen(parm);
85-
86-
if (op_len == 0) {
87-
*error_msg = "Content length is 0.";
88-
free(parm);
89-
return NULL;
90-
}
91-
92-
for (i = 0, x = 0; i < op_len; i++) {
93-
if (parm[i] == '|') {
94-
if (bin) {
95-
bin = 0;
96-
} else {
97-
bin = 1;
98-
}
99-
} else if(!esc && parm[i] == '\\') {
100-
esc = 1;
101-
} else {
102-
if (bin) {
103-
if (parm[i] == 0 || parm[i] == 1 || parm[i] == 2 ||
104-
parm[i] == 3 || parm[i] == 4 || parm[i] == 5 ||
105-
parm[i] == 6 || parm[i] == 7 || parm[i] == 8 ||
106-
parm[i] == 9 ||
107-
parm[i] == 'A' || parm[i] == 'a' ||
108-
parm[i] == 'B' || parm[i] == 'b' ||
109-
parm[i] == 'C' || parm[i] == 'c' ||
110-
parm[i] == 'D' || parm[i] == 'd' ||
111-
parm[i] == 'E' || parm[i] == 'e' ||
112-
parm[i] == 'F' || parm[i] == 'f')
113-
{
114-
bin_parm[bin_offset] = (char)parm[i];
115-
bin_offset++;
116-
if (bin_offset == 2) {
117-
c = strtol((char *)bin_parm, (char **) NULL, 16) & 0xFF;
118-
bin_offset = 0;
119-
parm[x] = c;
120-
x++;
121-
//converted = 1;
122-
}
123-
} else if (parm[i] == ' ') {
124-
}
125-
} else if (esc) {
126-
if (parm[i] == ':' ||
127-
parm[i] == ';' ||
128-
parm[i] == '\\' ||
129-
parm[i] == '\"')
130-
{
131-
parm[x] = parm[i];
132-
x++;
133-
} else {
134-
*error_msg = std::string("Unsupported escape sequence.").c_str();
135-
free(parm);
136-
return NULL;
137-
}
138-
esc = 0;
139-
//converted = 1;
140-
} else {
141-
parm[x] = parm[i];
142-
x++;
143-
}
144-
}
145-
}
146-
147-
#if 0
148-
if (converted) {
149-
op_len = x;
150-
}
151-
#endif
152-
153-
//processed = memcpy(processed, parm, op_len);
154-
processed = strdup(parm);
155-
free(parm);
156-
parm = NULL;
157-
158-
if (processed == NULL) {
159-
*error_msg = std::string("Error allocating memory for pattern matching content.").c_str();
160-
return NULL;
161-
}
162-
163-
return processed;
164-
}
37+
extern "C" {
16538

16639
/*
16740
*******************************************************************************

‎src/utils/acmp.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#endif
2424

2525
#include <cstddef>
26+
#include <string>
2627

2728

2829
extern "C" {
@@ -189,8 +190,6 @@ int acmp_process_quick(ACMPT *acmpt, const char **match, const char *data, size_
189190
*/
190191
int acmp_prepare(ACMP *parser);
191192

192-
char *parse_pm_content(const char *op_parm, unsigned short int op_len, const char **error_msg);
193-
194193
}
195194

196195
#endif /*ACMP_H_*/

0 commit comments

Comments
 (0)