search

zaproxy / zaproxy

The ZAP by Checkmarx Core project
https://www.zaproxy.org
Apache License 2.0
12.69k stars 2.27k forks source link

SQL Injection false positive #3662

Open JCWillard opened 7 years ago

JCWillard commented 7 years ago

I'm using ZAP 2.6, Standard mode. I have white-listed all the parameter inputs. Running Active Scan, I get a SQL Injection Alert that I just cannot understand:

When GETing the original page https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/search , I get my standard error page, a blank html page, as expected. This page requires another missing parameter in order to run and return data.

When GETing the page using the attack "query OR 1=1 -- " that is described in "Other Info" as: "The page results were successfully manipulated using the boolean conditions [query AND 1=1 -- ] and [query OR 1=1 -- ] The parameter value being modified was NOT stripped from the HTML output for the purposes of the comparison Data was NOT returned for the original parameter. The vulnerability was detected by successfully retrieving more data than originally returned, by manipulating the parameter"

The origianal URL and both of the URL copied from the alert (Attack 1 below) and the secondary attack (Attack 2 below) yield the exact same standard error page, as expected. There is no difference between all three of the html error pages returned. See the attached html error page here => OR 1=1.txt

Attack 1: https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/search?query=query+AND+1%3D1+--+

Attack 2: https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/search?query=query+OR+1%3D1+--+

If all three scenarios return a generic and equivalent error page, why is this calling this a successful attack?

JCWillard commented 7 years ago

Additional information: Here are the Request and Response

GET https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/search?query=query+AND+1%3D1+--+ HTTP/1.1 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0 Pragma: no-cache Cache-Control: no-cache Referer: https://localhost:8443/rcrainfoweb/action/modules/br/naics;jsessionid=B85BB70C2B1782BB8E31491393A2B8D8 Content-Length: 0 Host: localhost:8443

Response ***** HTTP/1.1 302 Found Server: Apache-Coyote/1.1 X-Frame-Options: deny X-XSS-Protection: 1 X-Content-Type-Options: nosniff Cache-Control: no-cache, no-store, must-revalidate, private Pragma: no-cache Set-Cookie: JSESSIONID=1C428B15C99E1835A43BF5D5A2E02559; Path=/rcrainfoweb/; Secure; HttpOnly Location: https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/rejectURL;jsessionid=1C428B15C99E1835A43BF5D5A2E02559 Content-Language: en-US Content-Length: 0 Date: Thu, 15 Jun 2017 01:15:42 GMT

JCWillard commented 7 years ago

Is my issue unclear?

psiinon commented 7 years ago

No, but we are all very busy and sometimes dont get to look at issues as quickly as we would like. Can you post the full error details, including the description and 'other info' field - they should explain why this was flagged as a potential issue.

JCWillard commented 7 years ago

Thanks for responding.

Description: SQL injection may be possible Other Info: When GETing the page using the attack "query OR 1=1 -- " that is described in "Other Info" as: "The page results were successfully manipulated using the boolean conditions [query AND 1=1 -- ] and [query OR 1=1 -- ] The parameter value being modified was NOT stripped from the HTML output for the purposes of the comparison Data was NOT returned for the original parameter. The vulnerability was detected by successfully retrieving more data than originally returned, by manipulating the parameter"

As I understand the method that ZAP uses for this SQL Injection attack: 1.)query a page 2.) query the page with "query AND 1=1--" and 3.) query the page with "query OR 1=1--" and look for differences.

In ALL 3 steps, the response is a custom error page. (The original request fails because an additional parameter is required, but is not supplied).

The response in all three cases is:

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML+RDFa 1.1//EN">
<!--[if IEMobile 7]><html class="iem7 no-js" lang="en" dir="ltr"><![endif]-->
<!--[if lt IE 7]><html class="lt-ie9 lt-ie8 lt-ie7 no-js" lang="en" dir="ltr"><![endif]-->
<!--[if (IE 7)&(!IEMobile)]><html class="lt-ie9 lt-ie8 no-js" lang="en" dir="ltr"><![endif]-->
<!--[if IE 8]><html class="lt-ie9 no-js" lang="en" dir="ltr"><![endif]-->
<!--[if (gt IE 8)|(gt IEMobile 7)]><!--> <html class="no-js not-oldie" lang="en" dir="ltr" version="HTML+RDFa 1.1"
                                               xmlns:content="http://purl.org/rss/1.0/modules/content/"
                                               xmlns:dc="http://purl.org/dc/terms/"
                                               xmlns:foaf="http://xmlns.com/foaf/0.1/"
                                               xmlns:og="http://ogp.me/ns#"
                                               xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#"
                                               xmlns:sioc="http://rdfs.org/sioc/ns#"
                                               xmlns:sioct="http://rdfs.org/sioc/types#"
                                               xmlns:skos="http://www.w3.org/2004/02/skos/core#"
                                               xmlns:xsd="http://www.w3.org/2001/XMLSchema#"> <!--<![endif]-->

            <title></title>

    <body class="" >

        <section id="main-content" class="main-content clearfix" lang="en" role="main" tabindex="-1">
              <div class="main-column clearfix">
                  <br/>
              <h1 class="page-title"></h1>

            </div>
        </section>

</html>

If all three scenarios return a generic and equivalent error page, why is this calling this a successful attack?

thc202 commented 7 years ago

Is the false positive happening always?

kingthorin commented 7 years ago

What's the exact name of the alert?

JCWillard commented 7 years ago

SQL Injection Is that what you mean?

thc202 commented 7 years ago

Also, are you following the redirections when reproducing the attacks?

kingthorin commented 7 years ago

Is the response you quoted above the content of the 302 itself or the content of the redirect destination?

JCWillard commented 7 years ago

I'm no ZAP expert, so here's what I'm able to figure out to do: Under the Alerts tab, under the SQL Injection Alert, I see many items listed. I'm right-clicking one of the items and clicking "Open URL in browser". The response I see is my custom error page.

kingthorin, the quote above is the content of the custom error page.

JCWillard commented 7 years ago

Also, Under the Sites tab, I find the Get:search and next the Get:search(query). Get:search(query) has a red flag. The Request tab for Get:search is: GET https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/search HTTP/1.1 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 Accept: text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8 Accept-Language: en-US,en;q=0.5 Accept-Encoding: gzip, deflate, br Cookie: JSESSIONID=5C82D869448E30251FE15CB2AAD20A0D; _ceg.s=orlp5p; _ceg.u=orlp5p Connection: keep-alive Upgrade-Insecure-Requests: 1 Host: localhost:8443

The Request tab for Get:search(query) is: ET https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/search?query=query+OR+1%3D1+--+ HTTP/1.1 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:53.0) Gecko/20100101 Firefox/53.0 Accept: text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8 Accept-Language: en-US,en;q=0.5 Accept-Encoding: gzip, deflate, br Cookie: JSESSIONID=5C82D869448E30251FE15CB2AAD20A0D; _ceg.s=orlp5p; _ceg.u=orlp5p Connection: keep-alive Upgrade-Insecure-Requests: 1 Host: localhost:8443

The Response tab for both items shows the same response:

HTTP/1.1 302 Found Server: Apache-Coyote/1.1 X-Frame-Options: deny X-XSS-Protection: 1 X-Content-Type-Options: nosniff Cache-Control: no-cache, no-store, must-revalidate, private Pragma: no-cache Location: https://localhost:8443/rcrainfoweb/action/modules/br/interstateshiprecv/rejectURL Content-Language: en-US Content-Length: 0

thc202 commented 7 years ago

Are you able to consistently reproduce the issue?

PaulCanfield commented 7 years ago

I'm encountering what appears to be the same issue. When I run an active scan against any of my sites it will often trigger alerts indicating a potential SQL injection. Upon examination of the page I'm unable to find any evidence of vulnerability.

Quite vexing, as this has been happening quite frequently of late.

The alerts tab does not highlight or flag any of the response as indicative of a possible injection. In the headers and HTML output of the page I can find no evidence of any of the malicious parameters or anything out of the ordinary. In fact a HTML request with the malicious url and without any parameters yields the exact same result.

Here's the vexing thing, there's no connection to an SQL database, this is not a database driven site, yet something is causing ZAP to report that there's a potential SQL vulnerability.

Environment

ZAP Attack Proxy 2.6.0 compiled JAR OSX 10.9.5 Apache PHP 5.4

ALERT

Risk: High Parameter: query Attack: query" OR "1"="1" -- Evidence: CWE ID: 89 WASC ID: 19 Source: Active The page results were successfully manipulated using the boolean conditions [query" AND "1"="1" -- ] and [query" OR "1"="1" -- ] The parameter value being modified was NOT stripped from the HTML output for the purposes of the comparison Data was NOT returned for the original parameter. The vulnerability was detected by successfully retrieving more data than originally returned, by manipulating the parameter

Request Headers

GET https://sub.domain.com/path/creative?query=query%22+AND+%221%22%3D%221%22+--+ HTTP/1.1 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:39.0) Gecko/20100101 Firefox/39.0 Pragma: no-cache Cache-Control: no-cache Content-Length: 0 Referer: https://sub.domain.com/path/sitemap Host: sub.domain.com

Response Headers

HTTP/1.1 200 OK Date: Thu, 06 Jul 2017 22:05:52 GMT Server: Apache Strict-Transport-Security: max-age=31536000; Cache-Control: no-cache, no-store, must-revalidate, private Pragma: no-cache Vary: Accept-Encoding BALANCEDTO: PWEB24 X-Frame-Options: sameorigin X-XSS-Protection: 1; mode=block X-Content-Type-Options: nosniff Referrer-Policy: origin Content-Security-Policy: [HIDDEN] X-Content-Security-Policy: [HIDDEN] X-WebKit-CSP: [HIDDEN] Content-Type: text/html; charset=UTF-8 BALANCEDTO: CWEB2 Set-Cookie: PHPSESSID=kqunokqbbb8ucvgqc4ib323rk1; path=/; secure; HttpOnly;HttpOnly;Secure Set-Cookie: ROUTEID=.2; path=/ Set-Cookie: ROUTEID=.1;HttpOnly;path=/ Set-Cookie: ROUTEID=.1;HttpOnly;path=/ Set-Cookie: ROUTEID=.1;HttpOnly;path=/

RESPONSE BODY

<!DOCTYPE html>
<html>
    <head>
        <meta charset="UTF-8">
        <meta http-equiv="X-UA-Compatible" content="IE=edge">
        <meta name="viewport" content="width=device-width, initial-scale=1">
        <link rel="stylesheet" type="text/css" href="https://sub.domain.com/path/assets/f913c1d2/css/bootstrap.css" />
<link rel="stylesheet" type="text/css" href="https://sub.domain.com/path/assets/f913c1d2/css/jquery-ui.min.css" />
<link rel="stylesheet" type="text/css" href="https://sub.domain.com/path/assets/f913c1d2/css/creative.css" />
<script type="text/javascript" src="https://sub.domain.com/path/assets/f913c1d2/js/jquery.min.js"></script>
<script type="text/javascript" src="https://sub.domain.com/path/assets/f913c1d2/js/bootstrap.min.js"></script>
<script type="text/javascript" src="https://sub.domain.com/path/assets/f913c1d2/js/jquery-ui.min.js"></script>
<title>Creative</title>
    </head>
    <body>
        <div class="container" id="main">
            <div class="masthead">
                <ul class="nav nav-pills pull-right">
                                        <li class="active">
                        <a href="/path/creative" title="pages">Preview</a>
                    </li>
                </ul>
                <h3 class="pageTitle muted">Creative</h3>
            </div>
            <hr/>

<p>
    <em>Following these links will set the site into preview mode; intended to review layout, copy and templates, some site functions will not 
        work properly and dynamic content will be populated with test data.     </em>
</p>

<p>
    <em>
        <b><a class="showTestingVars">Set Testing Vars</a></b>
    </em>
    <div class="testingVars">
        <a class="hideTestingVars">close <img src="/path/assets/f913c1d2/images/icons/stop.png" /></a>
        <div id="testVarsFormContainer">
    <form id="testingVarsForm" method="get">
                    <div class="row">
                <label>Language:</label>
                <select name="language">
                                                                    <option selected="selected"  value="en_us">English</option>
                                    </select>
                <br />
            </div>

        <div class="row">
            <label>Time Machine: </label>
            <input type="text" id="timepicker" name="timeMachineHMS" value="14:56:27" />            
            <input type="text" id="datepicker" name="timeMachine" value="07/06/2017" /> 
        </div>

        <div class="row">
            <label>Email: </label>
            <input name="emailMachine" value="" />
        </div>

        <div class="submitButton row" style="margin-bottom: 0px">
            <input type="submit" value="Set Cookies" class="btn btn-primary" />
        </div>
    </form>
</div>  </div>      
</p>
<hr />
            <h4>Main</h4>
        <ul>
                                                <li>
                        <a target="_blank" href="/path/creative/load?route=%2F" >Landing</a>
                    </li>
                                    </ul>
        </div>
    <script type="text/javascript" src="https://sub.domain.com/path/assets/57a5ed0f/js/main.js"></script>
<script type="text/javascript" src="https://sub.domain.com/path/assets/f913c1d2/js/creative.js"></script>
</body>
</html>
PaulCanfield commented 7 years ago

This is still and issue and I'm able to reproduce is consistently, is this a known issue, is there already a resolution for it?

kingthorin commented 7 years ago

@PaulCanfield this is not a known issue (beyond what is discussed in this ticket/thread), there is not yet any resolution.

The code is available here: https://github.com/zaproxy/zap-extensions/blob/master/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/TestSQLInjection.java if someone has time/interest in tackling a fix. Any potential/proposed fix should probably also verify before and after states/results via wavsep SQLi. (ref: https://zapbot.github.io/zap-mgmt-scripts/reports/wavsep-1.5-2.6.0-All-M-M.html) https://github.com/zaproxy/zaproxy/blob/develop/CONTRIBUTING.md https://github.com/zaproxy/zaproxy/wiki/Contributing-Changes https://github.com/zaproxy/zaproxy/wiki/DevGuidelines

kingthorin commented 7 years ago

@PaulCanfield @JCWillard are either of you able to provide both the original good response and the response to the injection? Or a mock-up of any sort that exhibits the issue?

Without solid details or a good test case there's not much we can do to address this FP issue.

alxleonov commented 6 years ago

@kingthorin hi, I have a very similar issue and was able to reliably reproduce it with 2.6.0. It also manifests as a 'Remote OS Command Injection' false positive with exactly the same symptoms.

First, here's the setting:

Now, when ZAP is doing an active scan for the form to create objects, it does not understand that creating them (or changing or deleting them, for that matter) will change the page showing a list of them.

Therefore, when the first thread sends a payload, which, it expects, will change the page (as a result of an SQL injection), and then another thread creates an objects thus changing the page, then the first thread thinks it has succeeded, creating an alert.

This problem (at least, for me) reliably disappears if the number of parallel scanning threads is reduced to 1.

Hope this helps!

kingthorin commented 6 years ago

This is a good point. I know of at least one commercial scanner that rechecks critical findings like this in single threaded mode to rule out such conditions.

rinogo commented 6 years ago

I'm seeing this issue as well.

To investigate further, I right click on the alert and select "Open/Resend with Request Editor..."

I then use the "Manual Request Editor" to "Send" the request again and manually inspect the response. I don't see any evidence of SQL injection. To be totally safe, I remove the parameter that ZAP was attacking and "Send" the request one more time. Diffing the responses reveals that they are identical. This indicates that there is actually no evidence that the alert is valid, correct?

For me, this alert happened on a GET request to object X in our REST API. I suppose this false positive could be avoided by modifying the URLs that will be attacked, correct? In other words, we could only attack object X via GET and reserve all POST attacks for object Y. This would prevent a multithreaded POST request from generating a false positive on a GET of object X. Does this seem like a sound strategy?

meetinthemiddle-be commented 5 years ago

I can confirm that this still happens in ZAP 2.8, e.g. on URLs like https://xyz.com/core/*.css$ where the SQL payload is sent in a cookie with the request.

krptodr commented 4 years ago

This happens for me when the injection is set in the referer header.

jhowlett96 commented 3 years ago

I am having high numbers (300+) of SQL injection false positives due to this and I now have to completely ignore SQL injection alerts. I can confirm that they are false positives because there are 30+ alerts for one page and they are all the values ZAP knows of (cookies, html form inputs etc) with some of the alerts due to cookies that are not even used on the page. Also some pages that are flagged for SQLi do not even touch the database. The site I am testing does has many places where the database is altered and objects created giving different responses as ZAP creates these objects.

Two solutions came to mind:

  1. Could there be a way for putting a flag on each rule that would indicate to ZAP that these rules need reconfirming in a single thread?

  2. Slightly simpler. Could there be a way for putting a flag on each rule to tell ZAP to only use one thread for these specified tests?

psiinon commented 3 years ago

Good suggestions but they would require code changes. A possible workaround that should work right now - run the SQL injection tests in a separate scan and configure ZAP to only use one thread for that scan.

jhowlett96 commented 3 years ago

Thanks @psiinon for getting back to me so quickly. I can confirm that running the SQLi rule in a single thread removes all false positives and your work around allows me to generate one report of all alerts from two scans (single thread for SQLi and multi-thread for all other rules).

sgerlach commented 2 years ago

Can confirm that this logic test in Plugin 40018 with 'AND 1=1' / 'OR 1=1' is generating a lot of false positives pretty constantly. Very reproducible with javapringvulny Something is going awry with the page content check, though I haven't been able to nail it down exactly.