Is this code code vulnerable to XSS?

Question

I have a function that accepts the param url and passes it to exec()

function getQueryValue(url, name) {
    name = name.replace(/[\[]/, "\\[").replace(/[\]]/, "\\]");
    var results = new RegExp("[\\?&]" + name + "=([^&#]*)").exec(url);
    if (results) return decodeURIComponent(results[1].replace(/\+/g, " "))
}

Then later, I call getQueryValue and supply location.href which would then get passed into exec().

function try_login() {
    <snip>
    var redir = getQueryValue(location.href, "redir");
    redir && (xstr += "&redir=" + encodeURIComponent(redir)), clear_error_status_line(), disable_input();
    <snip>
}

If location.href can be controlled by the user then wouldn't this be vulnerable to some form of DOM based cross-site scripting?

Answer 1

The exec in the first block isn't a problem; that's a member of the RegExp (Regular Expression) object, and it takes the string (in this case, the URL) upon which to execute (evaluate) the regular expression. Regexes can be dangerous - in particular, they're a common cause of denial-of-service attacks if the attacker can control at least one of the regex and the input - but I don't expect that to be a problem here (and it's client-side code so all it would do is make the user's CPU burn a ton of cycles). Regexes are also sometimes used in (usually doomed) attempts at preventing XSS - in particular, almost every WAF is, at its heart, just a pile of deeply insufficient regexes that are trivial to bypass - and I thought that's what you were asking about here but it isn't.

I don't see any way this is likely to be an XSS risk directly. The function looks for a URL parameter called "redir", extracts the value, checks if the value is truthy, and if so appends it to xstr. I don't know what else is in xstr (from context, it's a URL), or what it gets used for, though. It could be a problem if xstr is later injected into a JS block (encodeURIComponent is not JS-safe, in particular it does not encode ' which means you could break out of an apostrophe-delimited string) but that's pretty unlikely.

The biggest risk I see is that it obviously could lead to an open redirect, which is bad in and of itself but in particular can be used for XSS on some browsers, by redirecting to a javascript: or data: URI that are interpreted in the context of the original origin. (Some browsers either won't allow such redirects or will put them in a null or isolated origin, in which case the injected script can't do anything interesting.)