Cross Site Scripting issue with document.write

Question

I am working with some old code which has been reported is vulnerable to cross site scripting. The line of code is

document.write("<input type=hidden name=field1 value='" + getCookieValue('fieldval') + "' />");

The report also gave the following example on how malicious code can be injected into the page. By updating the cookie value as

fieldval='><img src=x onerror=alert(1)>

Can anybody provide insights on how this vulnerability can be fixed?

not sure if this is the issue but, you need to scape the quotes and double quotes. — Toping
How would anybody inject malicious code into your cookies? Of course, escaping it wouldn't be bad. — Bergi
@Bergi I imagine the user just edited the cookies in the browser. I'm not sure how this would affect other users though, so I don't see it posing too much problem in the grand scheme of things. However, whatever coding practices lead to this XSS, are likely present elsewhere as well. — Julio
@Louis: Every user can execute malicious code on his own machine. So as long as one cannot influence the cookies sent to other users, this is not a security issue. — Bergi
@Bergi correct, I indicated as much. However, the poor practices that lead to this issue are likely present in other parts of the code. Parts that may affect other users. — Julio

Julio Julio · Accepted Answer · 2013-02-07T19:27:11

You're going to need to validate the data coming from getCookieValue. If you're expecting a number, ensure that the value returned is numeric. Also ensure that any escape characters (e.g. quotes that break out of your javascript) are not present in the field. A fix for this would look like:

function is_valid(value) {
     // Do some check here depending on what you're expecting.
     // I also recommend escaping any quotes (i.e. " becomes \")
     // Ideally, you'd just whitelist what is acceptable input (A-Z0-9 or whatever,
     // and return false from this function if something else is present in 
     // value!)
}

var cookie_value = getCookieValue('fieldval');

if(is_valid(cookie_value)) {
    document.write('<input type="hidden name="field1" value="' + cookie_value + '" />');
}

Long story short, sanitize the data before you document.write or you end up with a reflected XSS.

As mentioned in the comments above, an XSS originating from a user's own cookies (something they modify themselves) is not particularly worrisome. However, whatever coding practices lead to this are likely present elsewhere. I'd recommend reviewing your source and ensuring that all input from users is treated as untrusted and sanitized appropriately.

Cross Site Scripting issue with document.write

2 Answers