CODEWITHSUNDEEP
javascriptxss
fn79
fn79

Reputation: 303

Cross Site Scripting issue with document.write

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?

Upvotes: 3

Views: 10725

Answers (2)

crackmigg
crackmigg

Reputation: 5901

Your code contains two mistakes:

  1. You insert untrusted data into the output without encoding it correctly (which opens the door for reflected XSS attacks)
  2. You use document.write to insert plain HTML into the DOM (which opens the door for DOM XSS attacks)

Before reinventing the wheel you should check out the OWASP cheat sheets to correct the mistakes:

  1. OWASP XSS Prevention Cheak Sheet - Rule 2
  2. OWASP DOM based XSS Prevention Cheat Sheet - Rule 2

As you can see, your problem is not fixed by only escaping the quotes. Whitelisting your untrusted data is always the preferred way and a valid advice. For further reading about XSS in general the links contain many references.

Upvotes: 2

Julio
Julio

Reputation: 2290

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.

Upvotes: 2

Related Questions