Skip to content
Browse files

fixed xss vulnerabilities on .attr() and .text()

  • Loading branch information...
1 parent e9f8e00 commit 1e2b3240f11b1a7fa3f0bc9009f18a5d7b59bae1 Matthew Mueller committed Jul 5, 2012
Showing with 26 additions and 5 deletions.
  1. +1 −1 lib/api/manipulation.js
  2. +1 −0 lib/api/parse.js
  3. +11 −4 lib/api/utils.js
  4. +6 −0 test/api.attributes.js
  5. +7 −0 test/api.manipulation.js
View
2 lib/api/manipulation.js
@@ -202,7 +202,7 @@ var text = exports.text = function(str) {
}
var elem = {
- data : str,
+ data : $.encode(str),
type : 'text',
parent : null,
prev : null,
View
1 lib/api/parse.js
@@ -3,6 +3,7 @@
*/
var htmlparser = require('htmlparser2'),
_ = require('underscore'),
+ entities = require('entities'),
$ = require('../cheerio'),
isArray = Array.isArray;
View
15 lib/api/utils.js
@@ -34,6 +34,10 @@ _.each(types.split(' '), function(name) {
*/
var tags = { tag : true, script : true, style : true };
+// Expose encode and decode methods from FB55's node-entities library
+var encode = exports.encode = entities.encode;
+var decode = exports.decode = entities.decode;
+
/*
isTag(type) includes <script> and <style> tags
*/
@@ -198,11 +202,13 @@ var attr = exports.attr = function( elem, name, value, pass ) {
$.removeAttr(elem, name);
} else {
// Set the attribute
- elem.attribs[name] = '' + value;
+ elem.attribs[name] = '' + $.encode(value);
}
+ } else if(elem.attribs[name]) {
+ // Get the (decoded) attribute
+ return $.decode(elem.attribs[name]);
} else {
- // Get the attributes
- return elem.attribs[name];
+ return undefined;
}
};
@@ -225,7 +231,7 @@ var text = exports.text = function(elems) {
for(var i = 0; i < len; i ++) {
elem = elems[i];
- if(elem.type === 'text') ret += entities.decode(elem.data, 2);
+ if(elem.type === 'text') ret += decode(elem.data, 2);
else if(elem.children && elem.type !== 'comment') {
ret += text(elem.children);
}
@@ -288,4 +294,5 @@ var root = exports.root = function() {
return $(this._root);
};
+
module.exports = $.extend(exports);
View
6 test/api.attributes.js
@@ -43,6 +43,12 @@ describe('$(...)', function() {
expect(attrs['data-url']).to.equal('http://apple.com');
});
+ it('(key, value) : should correctly encode then decode unsafe values', function() {
+ var $apple = $('.apple', fruits);
+ $apple.attr('href', 'http://github.com/"><script>alert("XSS!")</script><br');
+ expect($apple.get(0).attribs.href).to.equal('http://github.com/&quot;&gt;&lt;script&gt;alert(&quot;XSS!&quot;)&lt;/script&gt;&lt;br');
+ expect($apple.attr('href')).to.equal('http://github.com/"><script>alert("XSS!")</script><br');
+ });
});
describe('.removeAttr', function() {
View
7 test/api.manipulation.js
@@ -273,6 +273,13 @@ describe('$(...)', function() {
expect(text).to.equal('M&M');
});
+ it('(str) should encode then decode unsafe characters', function() {
+ var $apple = $('.apple', fruits);
+ $apple.text('blah <script>alert("XSS!")</script> blah');
+ expect($apple.get(0).children[0].data).to.equal('blah &lt;script&gt;alert(&quot;XSS!&quot;)&lt;/script&gt; blah');
+ expect($apple.text()).to.equal('blah <script>alert("XSS!")</script> blah');
+ });
+
});
});

0 comments on commit 1e2b324

Please sign in to comment.
Something went wrong with that request. Please try again.