Skip to content

Commit

Permalink
[fix bug 772998] Don't simply concat JS strings.
Browse files Browse the repository at this point in the history
- Add a k.safeString function similar to Python's markupsafe.escape.
- Add a k.safeInterpolate function that uses k.safeString.
- Use interpolation instead of concatenation in user autocomplete.
  • Loading branch information
James Socol authored and rlr committed Jul 16, 2012
1 parent 4a5e1e7 commit 9416717
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 3 deletions.
25 changes: 25 additions & 0 deletions media/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,31 @@ k = {};
return str;
}

var UNSAFE_CHARS = {
'&': '&',
'<': '&lt;',
'>': '&gt;',
"'": '&#39;',
'"': '&quot;'
};
k.safeString = function(str) {
return str.replace(new RegExp('[&<>\'"]', 'g'),
function(m) { return UNSAFE_CHARS[m]; });
};

k.safeInterpolate = function(fmt, obj, named) {
if (named) {
for (var j in obj) {
obj[j] = k.safeString(obj[j]);
}
} else {
for (var i=0, l=obj.length; i<l; i++) {
obj[i] = k.safeString(obj[i]);
}
}
return interpolate(fmt, obj, named);
};


// Pass CSRF token in XHR header
$.ajaxSetup({
Expand Down
25 changes: 25 additions & 0 deletions media/js/tests/commonutilstests.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,29 @@ test('no quotes', function() {
equals(k.unquote(s), s);
});

module('k.safeString');

test('escape html', function() {
var unsafeString = '<a href="foo&\'">',
safeString = '&lt;a href=&quot;foo&amp;&#39;&quot;&gt;';
equals(k.safeString(unsafeString), safeString);
});

module('k.safeInterpolate');

test('interpolate positional user input', function() {
var html = "<div>%s</div> %s",
unsafe = ["<a>", "<script>"],
safe = "<div>&lt;a&gt;</div> &lt;script&gt;";
equals(k.safeInterpolate(html, unsafe, false), safe);
});

test('interpolate named user input', function() {
var html = "<div>%(display)s <span>(%(name)s)</span></div>",
unsafe = {"display": "<script>alert('xss');</script>",
"name": "Jo&mdash;hn"},
safe = "<div>&lt;script&gt;alert(&#39;xss&#39;);&lt;/script&gt; <span>(Jo&amp;mdash;hn)</span></div>";
equals(k.safeInterpolate(html, unsafe, true), safe);
});

});
5 changes: 2 additions & 3 deletions media/js/users.autocomplete.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,9 @@ function init() {
resultsFormatter: function(item){
var term = $("#token-input-id_to").val();
if (item.display_name) {
return ("<li><div class='name_search'>" +
wrapTerm(item.display_name, term) + " [" +item.username + "]</div></div></li>");
return k.safeInterpolate('<li><div class="name_search">%(display_name)s [%(username)s]</div></li>', item, true);
}
return ("<li><div class='name_search'>" + item.username + "</div></li>");
return k.safeInterpolate('<li><div class="name_search">%(username)s</div></li>', item, true);
},
onAdd: function (item) {
$(this).closest('.single').closest('form').submit();
Expand Down

0 comments on commit 9416717

Please sign in to comment.