fix(jqLite): prevent possible XSS due to regex-based HTML replacement

This also splits the wrapping logic to one for modern browsers & one for IE 9
as IE 9 has restrictions that make it impossible to make it as secure.

Author: mgol

src/.eslintrc.json

@@ -100,6 +100,7 @@
     "VALIDITY_STATE_PROPERTY": false,
     "reloadWithDebugInfo": false,
     "stringify": false,
+    "UNSAFE_restoreLegacyJqLiteXHTMLReplacement": false,
 
     "NODE_TYPE_ELEMENT": false,
     "NODE_TYPE_ATTRIBUTE": false,

src/Angular.js

@@ -93,6 +93,7 @@
   hasOwnProperty,
   createMap,
   stringify,
+  UNSAFE_restoreLegacyJqLiteXHTMLReplacement,
 
   NODE_TYPE_ELEMENT,
   NODE_TYPE_ATTRIBUTE,
@@ -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.
  */

src/AngularPublic.js

@@ -156,6 +156,7 @@ function publishExternalAPI(angular) {
     'callbacks': {$$counter: 0},
     'getTestability': getTestability,
     'reloadWithDebugInfo': reloadWithDebugInfo,
+    'UNSAFE_restoreLegacyJqLiteXHTMLReplacement': UNSAFE_restoreLegacyJqLiteXHTMLReplacement,
     '$$minErr': minErr,
     '$$csp': csp,
     '$$encodeUriSegment': encodeUriSegment,

src/jqLite.js

@@ -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:
  *
@@ -169,20 +179,36 @@ var HTML_REGEXP = /<|&#?\w+;/;
 var TAG_NAME_REGEXP = /<([\w:-]+)/;
 var XHTML_TAG_REGEXP = /<(?!area|br|col|embed|hr|img|input|link|meta|param)(([\w:-]+)[^>]*)\/>/gi;
 
+// Table parts need to be wrapped with `<table>` or they're
+// stripped to their contents when put in a div.
+// XHTML parsers do not magically insert elements in the
+// same way that tag soup parsers do, so we cannot shorten
+// this by omitting <tbody> or other required elements.
 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 <10 only
+// IE 9 requires an option wrapper & it needs to have the whole table structure
+// set up up front; assigning `"<td></td>"` to `tr.innerHTML` doesn't work, etc.
+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);
@@ -203,7 +229,7 @@ function jqLiteHasData(node) {
 }
 
 function jqLiteBuildFragment(html, context) {
-  var tmp, tag, wrap,
+  var tmp, tag, wrap, finalHtml,
       fragment = context.createDocumentFragment(),
       nodes = [], i;
 
@@ -214,13 +240,30 @@ 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.firstChild;
+      }
+    } else {
+      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 = finalHtml;
     }
 
     nodes = concat(nodes, tmp.childNodes);

test/jqLiteSpec.js

@@ -147,6 +147,13 @@ describe('jqLite', function() {
       expect(nodes[0].nodeName.toLowerCase()).toBe('option');
     });
 
+    it('should allow construction of multiple <option> elements', function() {
+      var nodes = jqLite('<option></option><option></option>');
+      expect(nodes.length).toBe(2);
+      expect(nodes[0].nodeName.toLowerCase()).toBe('option');
+      expect(nodes[1].nodeName.toLowerCase()).toBe('option');
+    });
+
 
     // Special tests for the construction of elements which are restricted (in the HTML5 spec) to
     // being children of specific nodes.
@@ -169,6 +176,95 @@ 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;
+
+        // Support: IE <10
+        // In IE 9 we still need to use the old-style innerHTML assignment
+        // as that's the only one that works.
+        if (msie < 10) 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() {