search

owasp-modsecurity / ModSecurity

ModSecurity is an open source, cross platform web application firewall (WAF) engine for Apache, IIS and Nginx. It has a robust event-based programming language which provides protection from a range of attacks against web applications and allows for HTTP traffic monitoring, logging and real-time analysis.
https://www.modsecurity.org
Apache License 2.0
7.85k stars 1.56k forks source link

Repeated headers cause problems in json audit logs #2777

Open jbohanon opened 1 year ago

jbohanon commented 1 year ago

Describe the bug

Current implementation of json audit logs generates log entries with duplicated keys in the event of repeated headers in either the request or response. This is technically not invalid json per RFC 7159 Section 4, but most implementations silently drop all-but-first or all-but-last instances of duplicated keys. In some cases however, the behavior is undefined and an error is thrown. I found this issue in the nginx repo outlining a similar difficulty. The prevailing idea over there seemed to be to convert repeated header fields into a single array value containing all of the previously enumerated values. The HTTP specification states in RFC 9110 Section 5.2 that field values for repeated headers should be able to be concatenated delimited by commas. I think either of these are reasonable solutions with pros and cons...

Arrays Pros:

Cons:

Comma-delimited concatenated strings Pros:

Cons:

To Reproduce

curl -H "test: value" -H "test: another-value" "https://<site-behind-modsec>"

Server (please complete the following information):

Rule Set (please complete the following information):

martinhsv commented 1 year ago

Hi @jbohanon ,

Actually the issue that you found ( https://github.com/SpiderLabs/ModSecurity-nginx/issues/230 ) is exactly the same issue. Or, more precisely, that other issue is describing two sub-issues, one of which is the json-formatting issue for duplicates.

As you may gathered from my comments at https://github.com/SpiderLabs/ModSecurity-nginx/issues/230#issuecomment-743296353 , I do prefer the array format as the solution.