Protecting against XSS with dynamically generated JavaScript

Question

I have to pass a jQuery plugin some data. I could either pass it a URL where it will use a GET request to fetch the data, or directly pass it an array which will eliminate one server request. The data in question is user provided and not sanitized upon database entry.

The below script doesn't show the plugin, but does show how I might pass the data to the client so it may be directly passed to the plugin. As seen, the dynamically generated JS approach is suspect to XSS, however, the Ajax/JSON approach doesn't seem to be.

For this scenario, how should the dynamically generated JavaScript approach be secured, and is there risk to the Ajax/JSON approach?

<!DOCTYPE html>
<html>
    <head>
        <meta http-equiv="Content-Type" content="text/html; charset=utf-8">
        <title>XSS Testing</title>  
        <script src="getStuff.php?type=js" type="text/javascript"></script>
        <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.2/jquery.js" type="text/javascript"></script>
        <script type="text/javascript"> 
            $(function(){
                console.log(stuff);
                $.get( "getStuff.php", function( data ) {
                    console.log(data);
                },'json');
            });
        </script>
    </head>

    <body>
    </body> 
</html>

getStuff.php

<?php
$getFromDB=array(
    'Mary',
    'x"];alert(\'xss2\');x=["x',
    'John'
);
if(isset($_GET['type']) && $_GET['type']=='js') {
    header('Content-Type: application/javascript;');
    $s='var stuff=[';
    foreach($getFromDB as $row) {
        $s.='"'.$row.'",';
    }
    $s=substr($s, 0, -1).'];';
    echo($s);
}
else {
    header('Content-Type: application/json;');
    echo(json_encode($getFromDB));
}
?>

*"The data in question is user provided and not sanitized upon database entry."* Um, **all** data coming into the system from the outside world needs to be sanitised/checked prior to being recorded in the DB, in any reasonable system. If you aren't doing that, you will get bitten at some point by someone with a valid login to your system, but with nefarious intent. — T.J. Crowder, Apr 02 '15 at 17:08
@T.J.Crowder Really? http://stackoverflow.com/questions/11253532/html-xss-escape-on-input-vs-output http://lukeplant.me.uk/blog/posts/why-escape-on-input-is-a-bad-idea/ — user1032531, Apr 02 '15 at 17:30
Yeah, that was poorly worded, as we agreed elsewhere. Sorry 'bout that. I'm not saying **escape** it on input, I'm saying **check** it on input, for whether it looks reasonable. You escape on output, of course, [as I said below](http://stackoverflow.com/a/29418704/157247). — T.J. Crowder, Apr 02 '15 at 18:09

score 0 · Answer 1 · answered Apr 02 '15 at 16:26

0

if you expect to work with JSON, why not first and foremost verify that's what you're working with?

$.get(...)
 .success(function(data) {
    try {
      JSON.parse(data)
    } catch (e) {
      console.error("this isn't actually JSON");
    }
 })

JSON cannot contain functions, nor function calls, so just asking the browser to see if it can be parsed should be enough to make it go "there is stuff in here that isn't real JSON data".

The same goes for your PHP of course. Never build a string if you need a specific serialization. In this case, construct your key/map object the usual PHP way, and then use the built in json_encode function for converting that to a legal JSON serialization, instead.

answered Apr 02 '15 at 16:26

Mike 'Pomax' Kamermans

49,297
16
112
153

Thanks Mike, What do you mean by "specific serialization"? What about protecting with the dynamically generated JS approach? – user1032531 Apr 02 '15 at 16:37
Any "actually-has-a-spec" object serialization like JSON, HTML, XML, etc. Don't build those as strings, build them as structured object and then tell them to serialize themselves (if they can), or use a serialization API call. Even if you bypass the GET, you want to take that user data, and run it through a `JSON.parse`, so you can tell whether or not the data is actually plain JSON datatypes (objects, arrays, strings, numbers - no functions, no function calls) – Mike 'Pomax' Kamermans Apr 02 '15 at 16:40
Ah, you mean something like `exit('var stuff='.json_encode($getFromDB).';');`? I tested it and it doesn't execute the `alert` statement. Is this considered safe? – user1032531 Apr 02 '15 at 16:48
1

@Mike'Pomax'Kamermans *"if you expect to work with JSON, why not first and foremost verify that's what you're working with?"* He's already doing that `},'json');` if jQuery receives something that isn't json, it'l error. – Kevin B Apr 02 '15 at 17:20

T.J. Crowder · Answer 2 · 2015-04-02T17:25:31.727

It's almost like you've designed your example to be susceptible to hacking. You're doing nothing in the "js" case to ensure that the data is output with proper escaping, you're only doing it in the "json" case.

If you're going to include a JavaScript file that's purely data, like this:

<script src="getStuff.php?type=js"></script>

Then getStuff.php needs to ensure what it sends back is properly escaped as data:

<?php
$getFromDB=array(
    'Mary',
    'x"];alert(\'xss2\');x=["x',
    'John'
);
if(isset($_GET['type']) && $_GET['type']=='js') {
    header('Content-Type: application/javascript');
    echo('var data = ');
    echo(json_encode($getFromDB));
    echo(';');
}
else {
    header('Content-Type: application/json');
    echo(json_encode($getFromDB));
}
?>

And boom: No alert.

Side note: You shouldn't have the ; on the end of the Content-Type strings. I've removed them in the above.

Thanks TJ. I agree this approach works. I didn't originally know it would until seeing Pomax's response. — user1032531, Apr 02 '15 at 17:26

Protecting against XSS with dynamically generated JavaScript

2 Answers2