Is comparing unsanitized user input enough to be vulnerable to XSS?

Question

Say I receive user input from window.location.hash. Is comparing this unsanitized input to whitelisted values enough to open myself up to XSS vulnerabilities?

Take the following example code:

jQuery(function ($) { var hashVal = window.location.hash; $('.some-container').children().each(function(){ var link = $('a', $(this)); if(hashVal === link.attr('href')){ //do something with link } }); }); 

I've tested it against a few potentially troublesome strings with no success (or total success depending on how you look at it).

I'm curious if the if(hashVal === link.attr('href') could be exploited with something like #1 || true){}; //some malicious code...; //.

Answer 1

Anders' answer already explains why your code is fine. I'd like to concentrate on the following statement:

the if(hashVal === link.attr('href') could be exploited with something like #1 || true){}; //some malicious code...; //.

You seem to be assuming that the #1 || true){}; // some malicous code ; // would lead to the following:

if(#1 || true){}; //some malicious code...; // === link.attr('href') { // do something with link } 

(which wouldn't make much sense anyway because the #1 isn't legal javascript, as far as I know). However, that's not how Javascript (or any programming language I know) evaluates expressions.

To me, your example looks like you're trying to use an SQL injection attack on Javascript. SQL injection succeeds because people paste queries toegther like this (instead of using parametrized queries):

query = 'SELECT * FROM users WHERE loginname = "' + name + '"' 

This is a horrible blunder because name could contain something like 1"; drop table users; #.

Pasting together a query string like that lets an attacker escape from the protection of the quoted string into raw SQL.

But we aren't in the same position when writing Javascript code. In order to make the kind of exploit you feared possible in Javascript, you'd need to paste code together like people paste sql queries together, maybe like this:

var jscode = 'if (' + hash + ' === link.attr("href") { ... }' 

Then ifhash contained something like true) { malicious_code() }, and if as Anders said, you'd put the contents of jscode into the DOM or do eval(jscode), the malicious code would get executed.

Otherwise, it's nicely isolated inside a harmless string.

Answer 2

Your code looks fine, and it's not exploitable.

A JavaScript string just contains, well, a string of letters. Sometimes those letters form a piece of code. That is not dangerous in itself, as JavaScript only treats it as a collection of letters most of the time, e.g. when you do string comparison.

For it to open up a XSS vulnerability, something must execute the code. There are two ways that can happend:

• If you give the variable as input to a function that executes string as code. The main one here is eval, but there is also new Function, setTimeout and setInterval.
• If you put the string into the DOM, e.g. using innerHTML.

As long as you stay away from those two, the code will not be executed.