owasp-modsecurity / owasp-modsecurity/ModSecurity
Partial Log Forging in RuleMessage::log() via Unescaped Quote Characters in Request-Derived Fields
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 9.8k
- Forks
- 1.8k
- Avg merge
- 2h 46m
- Merged PRs (30d)
- 1
Description
Describe the bug
ModSecurity builds a single log via RuleMessage::log() line when a rule is triggered from several fields, some of which are individually passed through toHexIfNeeded(value, /escape_spec=/true) and some of which are not eg., hostname, URI, requestID. The final pass hex-escapes all bytes outside the printable ASCII range, which correctly neutralizes CR/LF-based log-line injection. However, because escape_spec defaults to false, the final pass does not protect against quotes and backlashes. This can result into these fields been inserted in [key "value"]`formatted log line without field-level quote escape.
Per my understanding, if (c < 32 || c > 126 || (escape_spec == true && (c == 34 || c == 92))
the c==34 and c ==92 which are double quote and backlash would only be escaped if escape_spec is true but my default in the function, it is set to false.
RuleMessage.cc fails to pass true for usercontrolled fields, and string.h requires true to escape quotes and backlash.
An attacker who can inject " into one of the fields likely breaks out of the [key "value"] log format.
Questionable Calls to toHexIfNeeded
return modsecurity::utils::string::toHexIfNeeded(msg);
ModSecurity/src/rule_message.cc
Line 94 in 7ea9fef
return modsecurity::utils::string::toHexIfNeeded(msg);
File: src/rule_message.cc
Function: RuleMessage::log(), RuleMessage::_details()
Root utility: modsecurity::utils::string::toHexIfNeeded() (src/utils/string.h)
Remediation
Pass escape_spec=true for all request-derived fields in RuleMessage::_details() (hostname, uri, msg, and ref), matching the treatment already given to rev, data, ver, and tags. Additionally, call the final toHexIfNeeded(msg) in RuleMessage::log() with escape_spec=true so quote/backslash neutralization is guaranteed at the sink regardless of upstream field-level omissions
Impact
an attacker who controls the value of one of the unescaped fields (likely host header) can embed a literal " followed by attacker-chosen bracketed content, causing the emitted log line to contain what appears to be additional, independent [key "value"] fields that were never actually produced by ModSecurity.
Lastly, if the logs are feed to other tools like SIEM or AI agents (which would be a fairly common use-case), this can cause unintended outcomes and may warrant a higher severity
Standalone POC
#include
#include
#include
#include
namespace modsecurity {
namespace utils {
namespace string {
// --- Verbatim copy of the escaping primitive under test (utils/string.h) --
inline std::string toHexIfNeeded(const std::string &str, bool escape_spec = false) {
// escape_spec: escape special chars or not
// spec chars: '"' (quotation mark, ascii 34), '' (backslash, ascii 92)
std::stringstream res;
for (const auto ch : str) {
int c = (unsigned char)ch;
if (c < 32 || c > 126 || (escape_spec == true && (c == 34 || c == 92))) {
res << "\x" << std::setw(2) << std::setfill('0') << std::hex << c;
} else {
res << ch;
}
}
return res.str();
}
} // namespace string
} // namespace utils
} // namespace modsecurity
// ---------------------------------------------------------------------------
// Minimal stand-in for RuleMessage::log() / RuleMessage::_details().
//
// The real code concatenates several "[key "value"]" fields (hostname,
// uri, unique_id, ...) into one line and, at the very end, calls:
// toHexIfNeeded(msg) // escape_spec defaults to false
// This reproduces exactly that pattern.
// ---------------------------------------------------------------------------
static std::string buildLogLine_vulnerable(const std::string &clientIp,
const std::string &hostname,
const std::string &uri) {
std::string msg;
msg += "[client " + clientIp + "] ";
msg += "ModSecurity: Access denied with code 403 (phase 2). ";
msg += "[hostname "" + hostname + ""] "; // hostname NOT pre-escaped
msg += "[uri "" + uri + ""] "; // uri NOT pre-escaped
// *** THE BUG ***
// Final pass strips control bytes (blocks CR/LF injection) but,
// since escape_spec is not passed as true, leaves '"' and '\'
// inside hostname/uri untouched.
return modsecurity::utils::string::toHexIfNeeded(msg /* escape_spec = false (default) */);
}
// Same line, but with the fix applied: every attacker-controlled field is
// individually run through toHexIfNeeded(field, /escape_spec=/true)
// BEFORE being wrapped in the "[key "value"]" quotes.
static std::string buildLogLine_fixed(const std::string &clientIp,
const std::string &hostname,
const std::string &uri) {
using modsecurity::utils::string::toHexIfNeeded;
std::string msg;
msg += "[client " + toHexIfNeeded(clientIp, true) + "] ";
msg += "ModSecurity: Access denied with code 403 (phase 2). ";
msg += "[hostname "" + toHexIfNeeded(hostname, true) + ""] ";
msg += "[uri "" + toHexIfNeeded(uri, true) + ""] ";
return msg;
}
int main() {
const std::string clientIp = "203.0.113.7";
std::cout << "=== Baseline: benign request ===\n";
{
std::string hostname = "www.example.com";
std::string uri = "/index.php";
std::cout << buildLogLine_vulnerable(clientIp, hostname, uri) << "\n\n";
}
std::cout << "=== Attacker-controlled Host header containing a double quote ===\n";
std::cout << "(no CR/LF used at all -- toHexIfNeeded already blocks those;\n"
" a single '\"' is enough because escape_spec defaults to false)\n\n";
{
// Attacker sends: Host: evil.example" ] [uri "/forged-by-attacker
std::string hostname = "evil.example\" ] [uri \"/forged-by-attacker";
std::string uri = "/index.php";
std::cout << "-- Current (vulnerable) call pattern --\n";
std::cout << " toHexIfNeeded(msg) // escape_spec defaults to false\n";
std::cout << buildLogLine_vulnerable(clientIp, hostname, uri) << "\n";
std::cout << " ^ Note there are now TWO \"[uri ...]\" fields -- the second\n"
" one was forged entirely by the attacker via the hostname field.\n\n";
std::cout << "-- Fixed call pattern --\n";
std::cout << " toHexIfNeeded(field, /*escape_spec=*/true) per attacker-controlled field\n";
std::cout << buildLogLine_fixed(clientIp, hostname, uri) << "\n";
std::cout << " ^ The quote is hex-escaped (\\x22) and the injected field stays\n"
" inert, literal content of the hostname value.\n";
}
return 0;
}
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with src/rule_message.cc, especially RuleMessage::log() and _details(), then read toHexIfNeeded() in src/utils/string.h. Compile or run the standalone POC to observe quote and backslash handling in request-derived fields. Done means the emitted log keeps injected quote-based fields inert while preserving the existing control-byte escaping behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 75/100