How is this code protecting against XSS attacks?

Question

I'm using a validation library that removes some common XSS attacks from the input to my web application. It works fine, and I'm also escaping everything I render to protect against XSS attacks.

The library contains this line in part of the XSS filtering process:

// Protect query string variables in URLs => 901119URL5918AMP18930PROTECT8198
str = str.replace(/\&([a-z\_0-9]+)\=([a-z\_0-9]+)/i, xss_hash() + '$1=$2');

xss_hash returns a string of random alpha-numeric characters. Basically it takes a URL with a query string, and mangles it a bit:

> xss('http://example.com?something=123&somethingElse=456&foo=bar')
'http://example.com?something=123eujdfnjsdhsomethingElse=456&foo=bar'

Besides having a bug (it only "protects" one parameter, not all of them), it seems to me the whole thing is itself a bug.

So my question is: what kind of attack vector is this kind of replacement protecting against?

If it's not really doing anything, I would like to submit a patch to the project removing it completely. And if it is legitimately protecting users of the library, I'd like to submit a patch to fix the existing bug.

Answer 1

xss_hash returns a string of random alpha-numeric characters.

Are they definitely random, or is it generated based on computable data?

It appears to be Security through obscurity: it's trying to replace all the &'s with xss_hash()'s so that the query is less readable. I'm guessing there is a part of the library which undoes this (i.e. treats all the xss_hash()'s in the string as &s for parsing purposes).

Answer 2

The code in question "protected query string variables" by replacing the & separating URL parameters with a random string, before doing some other processing that would remove or otherwise mangle ampersands. As Jay Shah pointed out, there was code just below that was meant to replace the query string ampersands, but another bug was preventing it from working as intended.