1

I have a spring-mvc web application which is "Active scanned" by ZAP tool. It has two High medium alert for SQL Injection which I believe is a false positive.

  1. The original URL is /msg/showList? which returns 200OK and json list of message. ZAP while running scan, adds a parameter /deliverymsg/showList?query=query+AND+1%3D1+--+ which also returns 200 OK and json list of message. There is no change in response, the list is same. The application doesn't read the param "query" added by ZAP, so there is no actual SQL Injection here. But ZAP alerts this as HIGH Medium.
  2. The original URL is /filterlist?fromDate=&toDate=&_csrf=1534403682524 that returns 200Ok and list , ZAP is scanning and adds the condition for SQL injection /filterlist?fromDate=&toDate=&_csrf=1534403682524+AND+1%3D1+--+ This parameter is csrf token added by spring-boot implicitly, again not actually read by application directly. But ZAP alerts this as High Medium again.

I want to understand how ZAP is working here, and how to fix this false positive so ZAP doesn't alert. I have had cases in past, where I changed the application code to pass the test. But unable to think a clear solution for this.

Am thinking of adding HandlerInterceptor and check request param for any "AND" word and return HTTP400, I do not want to do it, coz it will be just to fool ZAP in not alerting HIGH. I understand I can also give evidence of false positive and release this without fixing, but I cannot do that due to internal policies.

Am running ZAP in whatever default configuration it comes with.

Update:

I added HandlerInterceptor and rejected request having AND or query and so far re-running the ZAP for only the reported URL's didnt produce any alert. I wonder why is that? Because the URL's created by ZAP to attack has many more sql keywords like UNION ALL etc. I have only rejected request for two keywords.How could that solve the problem?

Why does ZAP appends it's own parameter query which application will never read, I dont understand the logic behind attacking with query.

Hima
  • 83
  • 1
  • 8
  • are you _sure_ that there is no change in response? That's the usual culprit – eis Aug 16 '18 at 10:28
  • No change. I compared the response json and header. No change. I have had an instance in past where HTTP response status was 404, and the body was different, coz of one timestamp param that changes everytime and that alerted ZAP. I fixed it by removing timestamp. But this time, the response body is the same. No change and so am clueless. – Hima Aug 16 '18 at 10:43
  • have you tried to [increase ZAP log levels](https://github.com/zaproxy/zaproxy/wiki/FAQlogging)? (you should probably do that when you are limiting ZAP to use just the specific rule to a specific url) – eis Aug 16 '18 at 10:45
  • No, I haven't. I will do that in the next run. This takes a lot of time to complete Active Scan. Any pointers, what should I look for specifically? Edit: Will only limit it to the URL. Thanks. – Hima Aug 16 '18 at 10:49

1 Answers1

1

It is possible that the findings are false positives. As you can imagine accounting for slight variances in app behavior across billions of potential web implementations is not exactly straight forward. Based on the details provided here it's hard to say. Usually for the SQLi alerts there is additional info in the "Other Info" section of the alert that may provide further clues as to ZAP's test and the observed "weirdness".

You can mark Alerts as False Positive in the UI (https://github.com/zaproxy/zap-core-help/wiki/HelpUiDialogsAddalert#confidence) or via the Web API. You can also install the "Context Alert Filters" addon and create rules in your context to set these as False Positive in the future. (Assuming you export/import your context.) [Further details on Context Alert Filters here > https://github.com/zaproxy/zap-extensions/wiki/HelpAddonsAlertFiltersAlertFilter]

ZAP's code is open source and publicly available so you can always look at the SQLi scanner (https://github.com/zaproxy/zap-extensions/blob/master/src/org/zaproxy/zap/extension/ascanrules/TestSQLInjection.java) and if you see an issue submit a PR with a fix, or open a new issue for the team: https://github.com/zaproxy/zaproxy/issues

I added HandlerInterceptor and rejected request having AND or query and so far re-running the ZAP for only the reported URL's didnt produce any alert. I wonder why is that? Because the URL's created by ZAP to attack has many more sql keywords like UNION ALL etc. I have only rejected request for two keywords.How could that solve the problem?

Well the alerts you received didn't have anything to do with the UNION injections did they? So simply preventing the issue that was being alerted upon (by filtering AND or query) you've hidden the behavior from ZAPs analysis. Further, the detection mechanism for boolean based SQLi and union based SQLi is different.

When you run an active scan that includes the Input Vector "URL Query String & Data Driven Nodes" then any request without a parameter will also be tried with the query parameter as that may sometimes uncover unknown handling (or mis-handling) within the web app, as well as some DOM XSS, etc. that may otherwise go undetected.

kingthorin
  • 1,419
  • 9
  • 18
  • Am not proud of adding `HandlerInterceptor` but I didn't know how to skip this `any request without a parameter will also be tried with the query parameter ` and release in the given timeframe. Another theory I need to test out is that ZAP is testing `create` and `get list` both and there could be a change in `get list` due to simultaneous `create` that I overlooked. I will compare again. I do not have the luxury YET to mark alerts as False positive due to business unit policies, so.. but that is my problem, not in scope of this question. Thank you for your answer. – Hima Aug 20 '18 at 11:26