Skip to content

Commit

Permalink
Security fixes - prevent possible XSS due to regex-based HTML replace…
Browse files Browse the repository at this point in the history
…ment

Updated To have angular#17028
  • Loading branch information
PPInfy committed Oct 5, 2020
1 parent e242e9b commit 4edfb09
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 20 deletions.
1 change: 1 addition & 0 deletions src/.eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
"VALIDITY_STATE_PROPERTY": false,
"reloadWithDebugInfo": false,
"stringify": false,
"UNSAFE_restoreLegacyJqLiteXHTMLReplacement": false,

"NODE_TYPE_ELEMENT": false,
"NODE_TYPE_ATTRIBUTE": false,
Expand Down
20 changes: 20 additions & 0 deletions src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -1967,6 +1967,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. Thus, if you need to call this function, please try to adjust
* your code for this change and remove your use of this function as soon as possible.
* Note that this only patches jqLite. If you use jQuery 3.5.0 or newer, please read the
* [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 @@ -155,6 +155,7 @@ function publishExternalAPI(angular) {
'callbacks': {$$counter: 0},
'getTestability': getTestability,
'reloadWithDebugInfo': reloadWithDebugInfo,
'UNSAFE_restoreLegacyJqLiteXHTMLReplacement': UNSAFE_restoreLegacyJqLiteXHTMLReplacement,
'$$minErr': minErr,
'$$csp': csp,
'$$encodeUriSegment': encodeUriSegment,
Expand Down
77 changes: 57 additions & 20 deletions src/jqLite.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,19 +170,26 @@ 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;

var wrapMapIE9 = {
option: [1, '<select multiple="multiple">', '</select>'],
_default: [0, '', '']
};
for (var key in wrapMap) {
var wrapMapValueClosing = wrapMap[key];
var wrapMapValue = wrapMapValueClosing.slice().reverse();
wrapMapIE9[key] = [wrapMapValue.length, '<' + wrapMapValue.join('><') + '>', '</' + wrapMapValueClosing.join('></') + '>'];
}
wrapMapIE9.optgroup = wrapMapIE9.option;

function jqLiteIsTextNode(html) {
return !HTML_REGEXP.test(html);
Expand All @@ -203,7 +210,7 @@ function jqLiteHasData(node) {
}

function jqLiteBuildFragment(html, context) {
var tmp, tag, wrap,
var tmp, tag, wrap, finalHtml,
fragment = context.createDocumentFragment(),
nodes = [], i;

Expand All @@ -214,13 +221,29 @@ 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];
finalHtml = JQLite.legacyXHTMLReplacement ?
html.replace(XHTML_TAG_REGEXP, '<$1></$2>') :
html;
if (msie < 10) {
wrap = wrapMapIE9[tag] || wrapMapIE9._default;
tmp.innerHTML = wrap[1] + finalHtml + wrap[2];

// Descend through wrappers to the right content
i = wrap[0];
while (i--) {
tmp = tmp.lastChild;
tmp = tmp.firstChild;
}
} else {
wrap = wrapMap[tag] || [];

// Create wrappers & descend into them
i = wrap.length;
while (--i > -1) {
tmp.appendChild(window.document.createElement(wrap[i]));
tmp = tmp.firstChild;
}

tmp.innerHTML = finalHtml;
}

nodes = concat(nodes, tmp.childNodes);
Expand Down Expand Up @@ -311,6 +334,23 @@ function jqLiteDealoc(element, onlyDescendants) {
}
}

function isEmptyObject(obj) {
var name;
for (name in obj) {
return false;
}
return true;
}
function removeIfEmptyData(element) {
var expandoId = element.ng339;
var expandoStore = expandoId && jqCache[expandoId];
var events = expandoStore && expandoStore.events;
var data = expandoStore && expandoStore.data;
if ((!data || isEmptyObject(data)) && (!events || isEmptyObject(events))) {
delete jqCache[expandoId];
element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it
}
}
function jqLiteOff(element, type, fn, unsupported) {
if (isDefined(unsupported)) throw jqLiteMinErr('offargs', 'jqLite#off() does not support the `selector` argument');

Expand Down Expand Up @@ -347,6 +387,7 @@ function jqLiteOff(element, type, fn, unsupported) {
}
});
}
removeIfEmptyData(element);
}

function jqLiteRemoveData(element, name) {
Expand All @@ -356,17 +397,12 @@ function jqLiteRemoveData(element, name) {
if (expandoStore) {
if (name) {
delete expandoStore.data[name];
return;
}
} else {

if (expandoStore.handle) {
if (expandoStore.events.$destroy) {
expandoStore.handle({}, '$destroy');
expandoStore.data = {};
}
jqLiteOff(element);
}
delete jqCache[expandoId];
element.ng339 = undefined; // don't delete DOM expandos. IE and Chrome don't like it

removeIfEmptyData(element);
}
}

Expand Down Expand Up @@ -616,6 +652,7 @@ forEach({
cleanData: function jqLiteCleanData(nodes) {
for (var i = 0, ii = nodes.length; i < ii; i++) {
jqLiteRemoveData(nodes[i]);
jqLiteOff(nodes[i]);
}
}
}, function(fn, name) {
Expand Down

0 comments on commit 4edfb09

Please sign in to comment.