5

I'm working through an OWASP Zap report that has flagged several URLs on the domain as being vulnerable to XSS, but the vulnerability is never output in a context that is executable by the browser. For instance, the report is showing 

path/contacts.php?search=John%3Balert%281%29

decoded: path/contacts.php?search=John;alert(1)

as a vulnerable URL.

The application does reflect this particular content in the response to the user:

var search = "John;alert(1)";

which I think is what triggers the Alert as an XSS attack in the application.

The XSS here is that an attacker could introduce whatever arbitrary code they wanted to in this context and have it reflected to the user's browser, but this code is never executed.

Testing the vulnerability manually, the application is converting characters in the attempted attack before outputting in the response (using PHP's htmlentities function), so something like

?search=John";alert(1);

gets returned as:

var search = "John";alert(1);";

So the question is, does this still qualify as an active XSS vulnerability?

Note: I have noted that there is still opportunity for proper validation of the input parameters, but my concern is the security implications here.

Conor Mancone
  • 29,899
  • 13
  • 91
  • 96
Noah Heck
  • 151
  • 3
  • 2
    False positives are common in basically all scanning tools. Automated tools simply aren't perfect, and will never be as smart as a trained professional. As a result any positive result from an automated scanner needs to be confirmed. That's just how it works :shrug: – Conor Mancone Apr 27 '20 at 17:17
  • 1
    Thanks @ConorMancone . That's kind of where my thinking was heading. To be clear though, the situation I presented should NOT be considered an XSS attack, correct? – Noah Heck Apr 27 '20 at 17:32
  • 1
    Zap, by default, does not execute javascript, so this one is due to the validation engine only checking the response page. I would recommend adding "zap" tag to your question - to get some attention from Zap devs for more details. – postoronnim Apr 27 '20 at 18:24

3 Answers3

5

This is very common for automated scanning tools. They can only be so smart, and so false positives can always happen (as can false-negatives for that matter). As a result any flagged vulnerability should be manually verified. This is why, for instance, bug bounty programs always have notices like "Results from automated scanners will not be considered" - it's easy to run a scanner and report a vulnerability, but security teams probably already do that anyway, so 99% of times it is a waste of time. However that leads to the next question:

Is this particular script handling this input properly?

That's a much trickier question to answer. You've already checked for the most obvious work around (injecting a double quote), but there are more options. The two that come to the top of my head is the mangling of character encoding and testing out backslashes. The former is a bit of a long-shot so I'll just focus on the latter:

  1. Try injecting a backslash at the end of your input ?search=whatever\
  2. If the application doesn't escape your backslash the javascript will be: var search = "whatever\";
  3. This will break the javascript. If you're lucky it will also let you inject XSS via a second parameter.

The last step could use an example. Imagine the full javascript was something like this:

var a="[search input]";var b="[name input]";

To be clear the code has been minimized to save bandwidth. This will be important. Also, neither escapes backslashes. Therefore you could put together a payload like this:

?search=\&name=;alert(1)//

You'll end up with this javascript:

var a="\";var b=";alert(1)//";

Which is a valid XSS payload. Your backslash escapes the closing double quote for the var a = " statement and so the starting double quote for the second variable ends up being a closing double quote. As a result your input for the second input ends up executing as plain javascript - you just need to end with a semi colon and then a comment character to get rid of the final closing double quote.

Now you probably won't have any luck getting this to work. If they also handle backslashes properly then you won't have any luck. If these lines are split into separate lines then you also won't have any luck (in most versions of javascript a line continuation character is required, but there are some non-standard browser behaviors here sometimes). However if they forgot to escape backslashes and you can find two inputs on the same line, then you have an easy XSS vulnerability.

Summary

Really though this highlights the main take away, which is why these things are so tricky: the kind of exploit required varies strongly on the context that the data gets injected into, and it is effectively impossible for automated scanners to test everything. Therefore as a penetration tester you have to be familiar with all your options, which is quite tricky! Fortunately though this is also why XSS is so common - programmers are equally bad at understanding all the ways in which they must be careful to protect against XSS vulnerabilities.

Conor Mancone
  • 29,899
  • 13
  • 91
  • 96
  • Great write-up Conor. Thanks. I'll keep on the lookout for the issue you pointed out. At this point, I'm happy to see the false positives as a way to look further into things. – Noah Heck Apr 28 '20 at 14:35
3

Yes, this does look like a false positive. Obviously you should double check with manual testing, as you are doing. Can you confirm that this is the only place on the page where the payload is reflected? I've raised this ZAP bug: https://github.com/zaproxy/zaproxy/issues/5958

Simon Bennetts
  • 1,390
  • 7
  • 10
  • 3
    Props to the ZAP team for being so involved here! –  Apr 28 '20 at 08:41
  • 1
    My Zap report is flagging 4 URLs with this behavior, and all of the instances I've looked into negates the "alert(1)" exploitation. I'll continue exploring today and update with additional details. Also, Props to the ZAP team for being so involved here! – Noah Heck Apr 28 '20 at 14:08
0

You test with John";alert(1); is syntactically incorrect. That's why even if there is vulnerability, nothing will happen. Browser will just report an error in JavaScript. Namely, the symbol " makes your JavaScript invalid.

To test how your client code behaves you should use valid JavaScript.

Use following JavaScript code alert(1); in your request:

path/contacts.php?search=alert%281%29

If this code is executed in your browser, this will mean there is a vulnerability. If not executed, this will not mean anything (it will not prove there is no vulnarability) and further analysis of your code will be needed.

mentallurg
  • 8,536
  • 4
  • 26
  • 41
  • Why is it incorrect? Why does the double quote make the JavaScript invalid. Looks like a perfectly reasonable payload to me, especially since it is being injected into a string context in JavaScript. – Conor Mancone Apr 27 '20 at 19:06
  • @ConorMancone: I didn't say payload incorrect. I said JavaScript. If there is a code in on the client side that produces JavaScript like `document.write(response);` then the resulting code containing single `"` will be syntactically incorrect. – mentallurg Apr 27 '20 at 19:09
  • I believe this is backend process, not front-end processing. I.e. javascript isn't reading the parameter and sending it to the page with `document.write`. Instead PHP is reading the parameter and writing it out to HTML. i.e. the OP said, `the application is converting characters in the attempted attack before outputting in the response (using PHP's htmlentities function)`. Therefore what he has written should work perfectly fine. – Conor Mancone Apr 27 '20 at 19:20
  • @ConorMancone: To *I believe this is backend process* - not really. We call it XSS when some untrusted source sends data to the backend, and backend includes these data into response instead of filtering them out or escaping them to make sure the code is not executable on the client. `htmlentities` can mitigate it. But the author performs test with syntactically incorrect JavaScript. That's why his test doesn't prove anything. – mentallurg Apr 27 '20 at 19:36
  • I think you're still missing my point, so let's just circle back to my first question: in what way is this JavaScript syntactically incorrect? – Conor Mancone Apr 27 '20 at 19:39
  • @ConorMancone: As I said, single `"` in the code makes it syntactically incorrect. If there is a code on the client that considers the response as JavaScript, then when browser tries to execute this peace of code an error will occur. To test if backend really filters or escapes input properly, one should use correct JavaScript, like `alert(123);` or `console.log(123);`. – mentallurg Apr 27 '20 at 19:52
  • Let us [continue this discussion in chat](https://chat.stackexchange.com/rooms/107277/discussion-between-conor-mancone-and-mentallurg). – Conor Mancone Apr 27 '20 at 20:28