NotionCommotion Posted October 9, 2014 Share Posted October 9, 2014 I have some PHP which sends some user provided data to the client: <?php header('Content-type: application/json'); $data=array( array('id'=>10,'firstname'=>'John','lastname'=>'Doe'), array('id'=>14,'firstname'=>'Jane','lastname'=>'Doe'), array('id'=>19,'firstname'=>'XSS!','lastname'=>'XSS!'), ); echo(json_encode($data)); ?> The client then displays the data: $.getJSON('getJSON.php', { something: 123 }, function(list) { var string = ''; for (var i in list) { string += '<li>< a href = "index.php?id=' + list[i]['id'] + '">' + list[i]['firstname'] + '</a></li>'; } $("#MyElem").html(string); }); Does this represent any XSS risk? If so, how do I prevent it? Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 10, 2014 Share Posted October 10, 2014 Yes, this is an XSS vulnerability. It has the same effect as dumping raw PHP values into HTML markup. The only difference is that your injection happens in the browser rather than on the server, which is called DOM-based XSS. So you must not inject raw values into any context. In the case of JavaScript, there are two secure alternatives: You generate a blank a element and then set the attributes and the content as text via attr() and text(). This is by far the most robust and foolproof solution. You manually HTML-escape the values. Neither JavaScript nor jQuery have a function for this, so you'd have to implement it yourself. Note that you also need to URL-encode the URL parameters. Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted October 10, 2014 Author Share Posted October 10, 2014 Thanks Jacques1, Your advice makes sense. Maybe I shouldn't be using jQuery/JavaScript to create this content directly, but use some sort of template system (P.S. I love Twig!). I assume it has methods to deal with XSS and encoding URLs. Given my brief research, sounds like HandleBars might be a good start. Do you have any advice? Thanks Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 10, 2014 Share Posted October 10, 2014 There's no need for a full-blown JS template engine unless you actually create lots and lots of dynamic HTML with JavaScript. For a bunch of dynamic elements, it's overkill. And, no, Handlebars.js doesn't automatically apply URL-encoding. It's not that smart (it's actually not context-aware at all). Quote Link to comment Share on other sites More sharing options...
NotionCommotion Posted October 10, 2014 Author Share Posted October 10, 2014 Thanks again Jacques1, I will stay away from JS template engines until I have a better need. In regards to my original question, is this safe? Note that I didn't use attr but used href. $(function(){ $.getJSON('getJSON.php', function(list) { var $MyElem = $("#MyElem"); for (var i in list) { $('<a/>', {href: 'index.php?id=' + encodeURIComponent(list[i]['id']),text: list[i]['firstname']}) .wrap("<li>").parent().appendTo('#MyElem'); }; }); }); Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted October 10, 2014 Share Posted October 10, 2014 Yes, this is safe. You should avoid plain for ... in loops, though, because they include inherited properties. If you or some library you've included happens to extend Object, then those extended properties will suddenly appear the the loop, even though you just wanted the properties from the JSON object. Use jQuery's $.each() instead or the classical hasOwnProperty() workaround. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.