Skip to content

Commit 50c3534

Browse files
committed
Fixed use after free in ModSecurity::processContentOffset
- Use after free issue detected with Address Sanitizer while running the reading_logs_with_offset example. - Keeps reference to last element in vars vector with vars.back(). Then it removes the element from vars calling vars.pop_back() which invalidates the reference, but it's accessed later in the function.
1 parent 7bff2f7 commit 50c3534

File tree

1 file changed

+10
-16
lines changed

1 file changed

+10
-16
lines changed

src/modsecurity.cc

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,11 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
258258
strlen("highlight"));
259259

260260
yajl_gen_array_open(g);
261-
while (vars.size() > 3) {
262-
std::string value;
261+
for(auto [it, pending] = std::tuple{vars.rbegin(), vars.size()}; pending > 3; pending -= 3) {
263262
yajl_gen_map_open(g);
264-
vars.pop_back();
265-
const std::string &startingAt = vars.back().str();
266-
vars.pop_back();
267-
const std::string &size = vars.back().str();
268-
vars.pop_back();
263+
it++;
264+
const std::string &startingAt = it->str(); it++;
265+
const std::string &size = it->str(); it++;
269266
yajl_gen_string(g,
270267
reinterpret_cast<const unsigned char*>("startingAt"),
271268
strlen("startingAt"));
@@ -284,7 +281,7 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
284281
return -1;
285282
}
286283

287-
value = std::string(content, stoi(startingAt), stoi(size));
284+
const auto value = std::string(content, stoi(startingAt), stoi(size));
288285
if (varValue.size() > 0) {
289286
varValue.append(" " + value);
290287
} else {
@@ -340,16 +337,13 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
340337

341338
yajl_gen_map_open(g);
342339

343-
while (ops.size() > 3) {
344-
std::string value;
340+
for(auto [it, pending] = std::tuple{ops.rbegin(), ops.size()}; pending > 3; pending -= 3) {
345341
yajl_gen_string(g, reinterpret_cast<const unsigned char*>("highlight"),
346342
strlen("highlight"));
347343
yajl_gen_map_open(g);
348-
ops.pop_back();
349-
std::string startingAt = ops.back().str();
350-
ops.pop_back();
351-
std::string size = ops.back().str();
352-
ops.pop_back();
344+
it++;
345+
const std::string &startingAt = it->str(); it++;
346+
const std::string &size = ops.back().str(); it++;
353347
yajl_gen_string(g,
354348
reinterpret_cast<const unsigned char*>("startingAt"),
355349
strlen("startingAt"));
@@ -371,7 +365,7 @@ int ModSecurity::processContentOffset(const char *content, size_t len,
371365
reinterpret_cast<const unsigned char*>("value"),
372366
strlen("value"));
373367

374-
value = std::string(varValue, stoi(startingAt), stoi(size));
368+
const auto value = std::string(varValue, stoi(startingAt), stoi(size));
375369

376370
yajl_gen_string(g,
377371
reinterpret_cast<const unsigned char*>(value.c_str()),

0 commit comments

Comments
 (0)