Partial Log Forging in RuleMessage::log() via Unescaped Quote Characters in Request-Derived Fields

[amitu314]

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;

}

[airween]

Hi @amitu314,

thanks for reporting this issue.

Unfortunately, this behavior is well known, and once I reported this to the previous maintainer, but he rejected my suggestions.

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

The other "problem" is that if we change the log format, that would break the parsers' behavior, which are for SIEM systems (as you mentioned).

But I plan to change this wrong behavior in next main version of libmodsecurity3.

(Note, that as I remember v2 also has some logging issues, where an attacker can confuse the SIEM system completely.)

[shishir-cyber]

Hi @airween, since a full format change is planned for the next major version, would a smaller opt-in fix (like escaping quotes only when an explicit flag is set, off by default to avoid breaking existing SIEM parsers) be useful as an interim step, or would you rather this wait for your rework?