Manipulation: Avoid concatenating strings in buildFragment

Concatenating HTML strings in buildFragment is a possible security risk as it
creates an opportunity of escaping the concatenated wrapper. It also makes it
impossible to support secure HTML wrappers like
[trusted types](https://web.dev/trusted-types/). It's safer to create wrapper
elements using `document.createElement` & `appendChild`.

The previous way was needed in jQuery <4 because IE <10 doesn't accept table
parts set via `innerHTML`, even if the element which contents are set is
a proper table element, e.g.:
```js
tr.innerHTML = "<td></td>";
```
The whole structure needs to be passed in one HTML string. jQuery 4 drops
support for IE <11 so this is no longer an issue; in older version we'd have
to duplicate the code paths.

IE <10 needed to have `<option>` elements wrapped in
`<select multiple="multiple">` but we no longer need that on master which
makes the `document.createElement` way shorter as we don't have to call
`setAttribute`.

All these improvements, apart from making logic more secure, decrease the
gzipped size by 58 bytes.

Closes gh-4724
Ref gh-4409
Ref angular/angular.js#17028

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>

diff --git a/src/manipulation/buildFragment.js b/src/manipulation/buildFragment.js
index daf383aea95189c38e6575d0dbcf6bce10c3f31a..9ac71acc932e26b1ec41c0493f302c18dc946110 100644 (file)
--- a/src/manipulation/buildFragment.js
+++ b/src/manipulation/buildFragment.js
@@ -1,6 +1,7 @@
 import jQuery from "../core.js";
 import toType from "../core/toType.js";
 import isAttached from "../core/isAttached.js";
+import arr from "../var/arr.js";
 import rtagName from "./var/rtagName.js";
 import rscriptType from "./var/rscriptType.js";
 import wrapMap from "./wrapMap.js";
@@ -35,15 +36,16 @@ function buildFragment( elems, context, scripts, selection, ignored ) {
 
                                // Deserialize a standard representation
                                tag = ( rtagName.exec( elem ) || [ "", "" ] )[ 1 ].toLowerCase();
-                               wrap = wrapMap[ tag ] || wrapMap._default;
-                               tmp.innerHTML = wrap[ 1 ] + jQuery.htmlPrefilter( elem ) + wrap[ 2 ];
+                               wrap = wrapMap[ tag ] || arr;
 
-                               // Descend through wrappers to the right content
-                               j = wrap[ 0 ];
-                               while ( j-- ) {
-                                       tmp = tmp.lastChild;
+                               // Create wrappers & descend into them.
+                               j = wrap.length;
+                               while ( --j > -1 ) {
+                                       tmp = tmp.appendChild( context.createElement( wrap[ j ] ) );
                                }
 
+                               tmp.innerHTML = jQuery.htmlPrefilter( elem );
+
                                jQuery.merge( nodes, tmp.childNodes );
 
                                // Remember the top-level container

diff --git a/src/manipulation/wrapMap.js b/src/manipulation/wrapMap.js
index 01937ecc35d8dddf00d6d690eb2cffa01f64ef83..457902595c34bd4107d5033eef23877354494e42 100644 (file)
--- a/src/manipulation/wrapMap.js
+++ b/src/manipulation/wrapMap.js
@@ -1,4 +1,3 @@
-// We have to close these tags to support XHTML (#13200)
 var wrapMap = {
 
        // Table parts need to be wrapped with `<table>` or they're
@@ -6,12 +5,10 @@ var wrapMap = {
        // 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.
-       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.tbody = wrapMap.tfoot = wrapMap.colgroup = wrapMap.caption = wrapMap.thead;

diff --git a/test/unit/manipulation.js b/test/unit/manipulation.js
index 81a64c76276955772651a1509ae9f69528e34394..a4a46f924adff321dd05151b89349f5988074c0b 100644 (file)
--- a/test/unit/manipulation.js
+++ b/test/unit/manipulation.js
@@ -2969,3 +2969,14 @@ QUnit.test( "Sanitized HTML doesn't get unsanitized", function( assert ) {
                test( "<noembed><noembed/><img src=url404 onerror=xss(12)>" );
        }
 } );
+
+QUnit.test( "Works with invalid attempts to close the table wrapper", function( assert ) {
+       assert.expect( 3 );
+
+       // This test case attempts to close the tags which wrap input
+       // based on matching done in wrapMap which should be ignored.
+       var elem = jQuery( "<td></td></tr></tbody></table><td></td>" );
+       assert.strictEqual( elem.length, 2, "Two elements created" );
+       assert.strictEqual( elem[ 0 ].nodeName.toLowerCase(), "td", "First element is td" );
+       assert.strictEqual( elem[ 1 ].nodeName.toLowerCase(), "td", "Second element is td" );
+} );