nbs-system / naxsi

NAXSI is an open-source, high performance, low rules maintenance WAF for NGINX
GNU General Public License v3.0
4.8k stars 606 forks source link

JSON Escape in JSON log #508

Closed marcinguy closed 4 years ago

marcinguy commented 4 years ago

My GROK rule had some problems. I realized JSON wasn't properly escaped.

GROK rule: ​myParsingrule %{date("yyyy/MM/dd HH:mm:ss"):connect_date} \[%{word:status}\] %{number:id}\#%{number:id2}: \*%{number:value} %{data::json}

I think only those needs to be escaped.

Examples, how it now works correctly:

curl http://127.0.0.1/\"a\?b=\<\>\\

2020/09/03 09:19:50 [error] 25894#0: *3 { "ip":"127.0.0.1","server":"127.0.0.1","uri":"/\"a","vers":"0.56","total_processed":"3","total_blocked":"3","config":"learning","cscore0":"$SQL","score0":"8","cscore1":"$XSS","score1":"24","cscore2":"$TRAVERSAL","score2":"4","zone0":"URL","id0":"1001","var_name0":"","zone1":"ARGS","id1":"1205","var_name1":"b","zone2":"ARGS","id2":"1302","var_name2":"b","zone3":"ARGS","id3":"1303","var_name3":"b" }, client: 127.0.0.1, server: localhost, request: "GET /"a?b=<>\ HTTP/1.1", host: "127.0.0.1"

curl http://127.0.0.1/\"\\a\?b=\<\>\\

2020/09/03 09:19:56 [error] 25894#0: *4 { "ip":"127.0.0.1","server":"127.0.0.1","uri":"/\"\\a","vers":"0.56","total_processed":"4","total_blocked":"4","config":"learning","cscore0":"$SQL","score0":"8","cscore1":"$XSS","score1":"24","cscore2":"$TRAVERSAL","score2":"8","zone0":"URL","id0":"1001","var_name0":"","zone1":"URL","id1":"1205","var_name1":"","zone2":"ARGS","id2":"1205","var_name2":"b","zone3":"ARGS","id3":"1302","var_name3":"b","zone4":"ARGS","id4":"1303","var_name4":"b" }, client: 127.0.0.1, server: localhost, request: "GET /"\a?b=<>\ HTTP/1.1", host: "127.0.0.1"

Please review, maybe it can be done better.

The alternative would be to use JSON library, but this would add a dependency, also not sure about the speed.

Thanks,

marcinguy commented 4 years ago

cc @wargio WDYT?

wargio commented 4 years ago

Hmm. I would say that the encoding is wrong. You should append a \ for each \ you find.

marcinguy commented 4 years ago

@wargio take a look at example requests. Could be confusing to see.

First request has zsh escaped " and second zsh escaped both " and \

Do you see it?

I append \ for each " and \

Or am I missing something?

marcinguy commented 4 years ago

@wargio FYI with GROK parsing it worked

You cannot escape the \ from " escaping backslash, if that is what you mean

wargio commented 4 years ago

No, what I mean is that you need to backslash each of the \ So /my/Uri/path/ becomes \/my\/uri\/ same for any other special chars.

marcinguy commented 4 years ago

Ahhh... Seems like you can, but don't have to, per specs:

https://stackoverflow.com/questions/1580647/json-why-are-forward-slashes-escaped#:~:text=JSON%20escapes%20the%20forward%20slash,%2Fb%2Fc%22%7D%20.

@wargio Let me know your decision. I can add it. Will be some code more however on possibly each request. I hope GROK parser will still work :) I need it for GROK parser.

marcinguy commented 4 years ago

@wargio Here is JSON RFC:

"characters that MUST be escaped: quotation mark, reverse solidus, and the control characters (U+0000 through U+001F)."

Source: https://datatracker.ietf.org/doc/rfc8259/?include_text=1

marcinguy commented 4 years ago

@wargio Latest changes seem to work correctly. As in first tests:

curl http://127.0.0.1/\"a\?b=\<\>\\

2020/09/03 09:19:50 [error] 25894#0: *3 { "ip":"127.0.0.1","server":"127.0.0.1","uri":"/\"a","vers":"0.56","total_processed":"3","total_blocked":"3","config":"learning","cscore0":"$SQL","score0":"8","cscore1":"$XSS","score1":"24","cscore2":"$TRAVERSAL","score2":"4","zone0":"URL","id0":"1001","var_name0":"","zone1":"ARGS","id1":"1205","var_name1":"b","zone2":"ARGS","id2":"1302","var_name2":"b","zone3":"ARGS","id3":"1303","var_name3":"b" }, client: 127.0.0.1, server: localhost, request: "GET /"a?b=<>\ HTTP/1.1", host: "127.0.0.1"

curl http://127.0.0.1/\"\\a\?b=\<\>\\

2020/09/03 09:19:56 [error] 25894#0: *4 { "ip":"127.0.0.1","server":"127.0.0.1","uri":"/\"\\a","vers":"0.56","total_processed":"4","total_blocked":"4","config":"learning","cscore0":"$SQL","score0":"8","cscore1":"$XSS","score1":"24","cscore2":"$TRAVERSAL","score2":"8","zone0":"URL","id0":"1001","var_name0":"","zone1":"URL","id1":"1205","var_name1":"","zone2":"ARGS","id2":"1205","var_name2":"b","zone3":"ARGS","id3":"1302","var_name3":"b","zone4":"ARGS","id4":"1303","var_name4":"b" }, client: 127.0.0.1, server: localhost, request: "GET /"\a?b=<>\ HTTP/1.1", host: "127.0.0.1"

Should me more optimal, only to call unescaping when needed for \ and/or for "

marcinguy commented 4 years ago

@wargio Added tests. But somehow the test plan complains on the amount of tests. Do you know how to fix it?

marcinguy commented 4 years ago

@wargio Fixed test. Tests are green now also

marcinguy commented 4 years ago

@wargio Changed per your requests. Tests seems green.