Special logging control characters not encoded when put into logs

[csanders-git]

From @WHK102 who posted on SpiderLabs/owasp-modsecurity-crs#268

For example: crs is connected to database log with exp reg to match content, injection sql by logs of crs example:

Change user agent to nikto and request: http://secure.com/"] [unique_id "');drop tables;-- -

Log output: [Mon Nov 16 13:49:08 2015] [error] [client x.x.x.x] ModSecurity: Access denied with code 403 (phase 2). Matched phrase "nikto" at REQUEST_HEADERS:User-Agent. [file "/home/x/crs/activated_rules/modsecurity_crs_35_bad_robots.conf"] [line "20"] [id "990002"] [rev "2"] [msg "Request Indicates a Security Scanner Scanned the Site"] [data "nikto 3.9"] [severity "CRITICAL"] [ver "OWASP_CRS/2.2.9"] [maturity "9"] [accuracy "9"] [tag "OWASP_CRS/AUTOMATION/SECURITY_SCANNER"] [tag "WASCTC/WASC-21"] [tag "OWASP_TOP_10/A7"] [tag "PCI/6.5.10"] [hostname "lapinturita.cl"] [uri "/"] [unique_id "');drop tables;-- -"] [unique_id "VkolJMBjCQ8AAH8FpyAAAAAg"]

[uri "/"] is validate as "/"
[unique_id "';drop tables;-- -"] is validate as ';drop tables;-- -
Now, save this with vulnerable code: insert into crs_logs ... values ('1', '');drop tables;-- -

Some people say that filter the field unique_id if only alphanumeric ?, the answer here.

Solution: display query string in urlencoded mode or escaped correctly the quotes to prevent spoof fields.

[csanders-git]

@dune73 followup:
@WHK102: Sorry for taking that long to address this. Let's sort this out!

Unfortunately, I am not totally sure about your initial request and your description is not 100% clear to me (I'm not a native speaker either).

Please tell me your request on a single line on it's own. Ideally as markdown code example. That way everything is clear.

What I get so far, this is a loginjection vulnerability, which mimics a ModSecurity log entry to execute an sql injection on the DB holding ModSecLogs. Is that interpretation correct?

[csanders-git]

My Followup:
@dune73 I am getting the same thing and the solution is to encode sensitive chars within the log ('[' and ']' ). I think we should address this but it would be a ModSecurity issue. It should also be possible to fuck with JSON logs in this way :-X @zimmerle keep this in mind for libmodsec :)

[p0pr0ck5]

Shouldn't it be the responsibility of the tooling used to write error logs into a database to perform that sanitation? WAF audit/error logs are inherently dangerous and need to be handled carefully. The case that @WHK102 provided seems rather specific to a single chunk of code designed to blindly ship data into a database with no regard for what's being touched.

[p0pr0ck5]

(It's also worth noting that centralizing audit/error log storage like this is exactly why existing ModSecurity already has JSON audit logging ;) )

[csanders-git]

@p0pr0ck5 totally agree. Mine and @dune73 comments are really not concerned with the SQL as much as the log injection, In v2 (with native) it is a concern for Apache/Nginx/IIS that before being put into the auditlog and error log that we don't encode the brackets. In v3 this will be more interesting as each log destination will need to do its own encoding. It is possible that the JSON stuff might want to encode {} as well :)

[p0pr0ck5]

YAJL already escapes curly braces as { and } respectively, otherwise we'd have broken JSON ;)

[csanders-git]

@p0pr0ck5 god bless YAJL :-)

[zimmerle]

Hi guys,

In my understanding it is the application that will import the logs that should be responsible to sanitize anything that may be potentially dangerous. It is impossible to know which character of combination is potentially dangerous to a given application when we don't know anything about it.
For instance, the SQL injection is not harmful for a noSQL database. Although, I understand the importance to keep the data parsable, such as keep the JSON in a readable state.

What do you think @csanders-git ?

[csanders-git]

In general @zimmerle i'd agree with you -- however we should certainly not allow things would alter the meaning of our own logs. So well escaping <script>alert(1);</script> should be left as an excerice to the importer ... perhaps things like newlines within data should be encoded to prevent someone from injecting another section. This changes annoying based on the mode that we allow so JSON filtering should be (and is) automatically done.

[dune73]

I'd favor a layered approach where we try to sanitize what we fill into the pipe and the log store on the receiving end does the same.

Other than that, I'm a bit at loss here.

[victorhora]

This is how audit logging with JSON for that request is currently presented in v3:

{"transaction":{"client_ip":"192.168.37.1","time_stamp":"Thu Oct 11 13:01:05 2018","server_id":"46dcb50d4ee6715c2376525ec11c49d6081aca9b","client_port":63438,"host_ip":"192.168.37.1","host_port":8080,"unique_id":"153927726598.322200","request":{"method":"GET","http_version":1.1,"uri":"/\"] [unique_id \"');drop tables;-- -","headers":{"Host":"localhost","User-Agent":"nikto","Content-Type":"plain","Content-Length":"0"}},"response":{"http_code":403,"headers":{"Server":"nginx/1.13.11","Date":"Thu, 11 Oct 2018 17:01:05 GMT","Content-Length":"170","Content-Type":"text/html","Connection":"keep-alive"}},"producer":{"modsecurity":"ModSecurity v3.0.2 (Linux)","connector":"ModSecurity-nginx v1.0.0","secrules_engine":"Enabled","components":["OWASP_CRS/3.0.2\""]},"messages":[{"message":"Found User-Agent associated with security scanner","details":{"match":"Matched \"Operator `PmFromFile' with parameter `scanners-user-agents.data' against variable `REQUEST_HEADERS:User-Agent' (Value: `nikto' )","reference":"o0,5v77,5t:lowercase","ruleId":"913100","file":"/root/owasp-modsecurity-crs-3.0/rules/REQUEST-913-SCANNER-DETECTION.conf","lineNumber":"17","data":"Matched Data: nikto found within REQUEST_HEADERS:User-Agent: nikto","severity":"2","ver":"OWASP_CRS/3.0.0","rev":"2","tags":["application-multi","language-multi","platform-multi","attack-reputation-scanner","OWASP_CRS/AUTOMATION/SECURITY_SCANNER","WASCTC/WASC-21","OWASP_TOP_10/A7","PCI/6.5.10"],"maturity":"9","accuracy":"9"}},{"message":"Inbound Anomaly Score Exceeded (Total Score: 5)","details":{"match":"Matched \"Operator `Ge' with parameter `5' against variable `TX:ANOMALY_SCORE' (Value: `5' )","reference":"","ruleId":"949110","file":"/root/owasp-modsecurity-crs-3.0/rules/REQUEST-949-BLOCKING-EVALUATION.conf","lineNumber":"36","data":"","severity":"2","ver":"","rev":"","tags":["application-multi","language-multi","platform-multi","attack-generic"],"maturity":"0","accuracy":"0"}},{"message":"Inbound Anomaly Score Exceeded (Total Inbound Score: 5 - SQLI=0,XSS=0,RFI=0,LFI=0,RCE=0,PHPI=0,HTTP=0,SESS=0): Found User-Agent associated with security scanner","details":{"match":"Matched \"Operator `Ge' with parameter `5' against variable `TX:INBOUND_ANOMALY_SCORE' (Value: `5' )","reference":"","ruleId":"980130","file":"/root/owasp-modsecurity-crs-3.0/rules/RESPONSE-980-CORRELATION.conf","lineNumber":"61","data":"","severity":"0","ver":"","rev":"","tags":["event-correlation"],"maturity":"0","accuracy":"0"}}]}}

Based on the discussion, I believe the general consensus is that:

a) YAJL takes care of escaping and validating the JSON before generating it. So JSON logging should be taken in consideration for environments concerning about log injection, although it's not flawless, it's a first layer into making the logs more resilient.

b) Blindly and unsafely importing WAF logs into a database is a bad idea regardless.