Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(jqLite): prevent possible XSS due to regex-based HTML replacement
Browse files Browse the repository at this point in the history
  • Loading branch information
mgol committed May 26, 2020
1 parent a31c207 commit 0b474b3
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
"VALIDITY_STATE_PROPERTY": false,
"reloadWithDebugInfo": false,
"stringify": false,
"UNSAFE_restoreLegacyJqLiteXHTMLReplacement": false,

"NODE_TYPE_ELEMENT": false,
"NODE_TYPE_ATTRIBUTE": false,
Expand Down
21 changes: 21 additions & 0 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
hasOwnProperty,
createMap,
stringify,
UNSAFE_restoreLegacyJqLiteXHTMLReplacement,
NODE_TYPE_ELEMENT,
NODE_TYPE_ATTRIBUTE,
Expand Down Expand Up @@ -1949,6 +1950,26 @@ function bindJQuery() {
bindJQueryFired = true;
}

/**
* @ngdoc function
* @name angular.UNSAFE_restoreLegacyJqLiteXHTMLReplacement
* @module ng
* @kind function
*
* @description
* Restores the pre-1.8 behavior of jqLite that turns XHTML-like strings like
* `<div /><span />` to `<div></div><span></span>` instead of `<div><span></span></div>`.
* The new behavior is a security fix so if you use this method, please try to adjust
* to the change & remove the call as soon as possible.
* Note that this only patches jqLite. If you use jQuery 3.5.0 or newer, please read
* [jQuery 3.5 upgrade guide](https://jquery.com/upgrade-guide/3.5/) for more details
* about the workarounds.
*/
function UNSAFE_restoreLegacyJqLiteXHTMLReplacement() {
JQLite.legacyXHTMLReplacement = true;
}

/**
* throw error if the argument is falsy.
*/
Expand Down
1 change: 1 addition & 0 deletions src/AngularPublic.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ function publishExternalAPI(angular) {
'callbacks': {$$counter: 0},
'getTestability': getTestability,
'reloadWithDebugInfo': reloadWithDebugInfo,
'UNSAFE_restoreLegacyJqLiteXHTMLReplacement': UNSAFE_restoreLegacyJqLiteXHTMLReplacement,
'$$minErr': minErr,
'$$csp': csp,
'$$encodeUriSegment': encodeUriSegment,
Expand Down
45 changes: 31 additions & 14 deletions src/jqLite.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,16 @@
* - [`val()`](http://api.jquery.com/val/)
* - [`wrap()`](http://api.jquery.com/wrap/)
*
* jqLite also provides a method restoring pre-1.8 insecure treatment of XHTML-like tags
* that makes input like `<div /><span />` turned to `<div></div><span></span>` instead of
* `<div><span></span></div>` like version 1.8 & newer do:
* ```js
* angular.UNSAFE_restoreLegacyJqLiteXHTMLReplacement();
* ```
* Note that this only patches jqLite. If you use jQuery 3.5.0 or newer, please read
* [jQuery 3.5 upgrade guide](https://jquery.com/upgrade-guide/3.5/) for more details
* about the workarounds.
*
* ## jQuery/jqLite Extras
* AngularJS also provides the following additional methods and events to both jQuery and jqLite:
*
Expand Down Expand Up @@ -170,19 +180,22 @@ var TAG_NAME_REGEXP = /<([\w:-]+)/;
var XHTML_TAG_REGEXP = /<(?!area|br|col|embed|hr|img|input|link|meta|param)(([\w:-]+)[^>]*)\/>/gi;

var wrapMap = {
'option': [1, '<select multiple="multiple">', '</select>'],

'thead': [1, '<table>', '</table>'],
'col': [2, '<table><colgroup>', '</colgroup></table>'],
'tr': [2, '<table><tbody>', '</tbody></table>'],
'td': [3, '<table><tbody><tr>', '</tr></tbody></table>'],
'_default': [0, '', '']
thead: ['table'],
col: ['colgroup', 'table'],
tr: ['tbody', 'table'],
td: ['tr', 'tbody', 'table']
};

wrapMap.optgroup = wrapMap.option;
wrapMap.tbody = wrapMap.tfoot = wrapMap.colgroup = wrapMap.caption = wrapMap.thead;
wrapMap.th = wrapMap.td;

// Support: IE <=9 only
// IE <=9 replaces <option> tags with their contents when inserted outside of
// the select element.
if (msie < 10) {
wrapMap.optgroup = wrapMap.option = [1, '<select multiple="multiple">', '</select>'];
}


function jqLiteIsTextNode(html) {
return !HTML_REGEXP.test(html);
Expand Down Expand Up @@ -214,15 +227,19 @@ function jqLiteBuildFragment(html, context) {
// Convert html into DOM nodes
tmp = fragment.appendChild(context.createElement('div'));
tag = (TAG_NAME_REGEXP.exec(html) || ['', ''])[1].toLowerCase();
wrap = wrapMap[tag] || wrapMap._default;
tmp.innerHTML = wrap[1] + html.replace(XHTML_TAG_REGEXP, '<$1></$2>') + wrap[2];
wrap = wrapMap[tag] || [];

// Descend through wrappers to the right content
i = wrap[0];
while (i--) {
tmp = tmp.lastChild;
// Create wrappers & descend into them.
i = wrap.length;
while (--i > -1) {
tmp.appendChild(window.document.createElement(wrap[i]));
tmp = tmp.firstChild;
}

tmp.innerHTML = JQLite.legacyXHTMLReplacement ?
html.replace(XHTML_TAG_REGEXP, '<$1></$2>') :
html;

nodes = concat(nodes, tmp.childNodes);

tmp = fragment.firstChild;
Expand Down
84 changes: 84 additions & 0 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,90 @@ describe('jqLite', function() {
expect(nodes[0].nodeName.toLowerCase()).toBe(name);
});
});

describe('security', function() {
it('shouldn\'t crash at attempts to close the table wrapper', function() {
// jQuery doesn't pass this test yet.
if (!_jqLiteMode) return;

expect(function() {
// This test case attempts to close the tags which wrap input
// based on matching done in wrapMap, escaping the wrapper & thus
// triggering an error when descending.
var el = jqLite('<td></td></tr></tbody></table><td></td>');
expect(el.length).toBe(2);
expect(el[0].nodeName.toLowerCase()).toBe('td');
expect(el[1].nodeName.toLowerCase()).toBe('td');
}).not.toThrow();
});

it('shouldn\'t unsanitize sanitized code', function(done) {
// jQuery <3.5.0 fail those tests.
if (isJQuery2x()) {
done();
return;
}

var counter = 0,
assertCount = 13,
container = jqLite('<div></div>');

function donePartial() {
counter++;
if (counter === assertCount) {
container.remove();
delete window.xss;
done();
}
}

jqLite(document.body).append(container);
window.xss = jasmine.createSpy('xss');

// Thanks to Masato Kinugawa from Cure53 for providing the following test cases.
// Note: below test cases need to invoke the xss function with consecutive
// decimal parameters for the assertions to be correct.
forEach([
'<img alt="<x" title="/><img src=url404 onerror=xss(0)>">',
'<img alt="\n<x" title="/>\n<img src=url404 onerror=xss(1)>">',
'<style><style/><img src=url404 onerror=xss(2)>',
'<xmp><xmp/><img src=url404 onerror=xss(3)>',
'<title><title /><img src=url404 onerror=xss(4)>',
'<iframe><iframe/><img src=url404 onerror=xss(5)>',
'<noframes><noframes/><img src=url404 onerror=xss(6)>',
'<noscript><noscript/><img src=url404 onerror=xss(7)>',
'<foo" alt="" title="/><img src=url404 onerror=xss(8)>">',
'<img alt="<x" title="" src="/><img src=url404 onerror=xss(9)>">',
'<noscript/><img src=url404 onerror=xss(10)>',
'<noembed><noembed/><img src=url404 onerror=xss(11)>',

'<option><style></option></select><img src=url404 onerror=xss(12)></style>'
], function(htmlString, index) {
var element = jqLite('<div></div>');

container.append(element);
element.append(jqLite(htmlString));

window.setTimeout(function() {
expect(window.xss).not.toHaveBeenCalledWith(index);
donePartial();
}, 1000);
});
});

it('should allow to restore legacy insecure behavior', function() {
// jQuery doesn't have this API.
if (!_jqLiteMode) return;

// eslint-disable-next-line new-cap
angular.UNSAFE_restoreLegacyJqLiteXHTMLReplacement();

var elem = jqLite('<div/><span/>');
expect(elem.length).toBe(2);
expect(elem[0].nodeName.toLowerCase()).toBe('div');
expect(elem[1].nodeName.toLowerCase()).toBe('span');
});
});
});

describe('_data', function() {
Expand Down

0 comments on commit 0b474b3

Please sign in to comment.