Attributes: Don't stringify attributes in the setter

mgol · web-flow · commit 4250b628783d · 2021-11-01T18:10:23.000+01:00
Stringifying attributes in the setter was needed for IE <=9 but it breaks trusted types enforcement when setting a script `src` attribute. Note that this doesn't mean script execution works. Since jQuery disables all scripts by changing their type and then executes them by creating fresh script tags with proper `src` & possibly other attributes, this unwraps any trusted `src` wrappers, making the script not execute under strict CSP settings. We might try to fix it in the future in a separate change. Fixes gh-4948 Closes gh-4949
diff --git a/src/attributes/attr.js b/src/attributes/attr.js
@@ -50,7 +50,7 @@ jQuery.extend( {
 				return ret;
 			}
 
-			elem.setAttribute( name, value + "" );
+			elem.setAttribute( name, value );
 			return value;
 		}
 
diff --git a/test/data/mock.php b/test/data/mock.php
@@ -247,6 +247,12 @@ protected function trustedHtml( $req ) {
 		echo file_get_contents( __DIR__ . '/trusted-html.html' );
 	}
 
+	protected function trustedTypesAttributes( $req ) {
+		header( "Content-Security-Policy: require-trusted-types-for 'script'; report-uri ./mock.php?action=cspLog" );
+		header( 'Content-type: text/html' );
+		echo file_get_contents( __DIR__ . '/trusted-types-attributes.html' );
+	}
+
 	protected function errorWithScript( $req ) {
 		header( 'HTTP/1.0 404 Not Found' );
 		if ( isset( $req->query['withScriptContentType'] ) ) {
diff --git a/test/data/trusted-types-attributes.html b/test/data/trusted-types-attributes.html
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<head>
+	<meta charset=utf-8 />
+	<title>Trusted HTML attribute tests</title>
+</head>
+<body>
+<div id="qunit-fixture"></div>
+<script src="../../dist/jquery.js"></script>
+<script src="iframeTest.js"></script>
+<script>
+	var i, input, elem, policy,
+		results = [];
+
+	function runTests( messagePrefix, getTrustedScriptUrlWrapper ) {
+		try {
+			elem = jQuery( "<script><\/script>" )
+				.attr( "src", getTrustedScriptUrlWrapper( "trusted-types-attributes.js" ) );
+			elem.appendTo( document.body );
+
+			results.push( {
+				actual: elem.attr( "src" ),
+				expected: "trusted-types-attributes.js",
+				message: messagePrefix + ": script URL properly set"
+			} );
+		} catch ( e ) {
+			results.push( {
+				actual: "error thrown",
+				expected: "",
+				message: messagePrefix + ": error has been thrown"
+			} );
+		}
+	}
+
+	if ( typeof trustedTypes !== "undefined" ) {
+		policy = trustedTypes.createPolicy( "jquery-test-policy", {
+			createScriptURL: function( html ) {
+				return html;
+			}
+		} );
+
+		runTests( "TrustedScriptURL", function wrapInTrustedScriptUrl( input ) {
+			return policy.createScriptURL( input );
+		} );
+	} else {
+
+		// No TrustedScriptURL support so let's at least run tests with object wrappers
+		// with a proper `toString` function. See trusted-html.html for more context.
+		runTests( "Object wrapper", function( input ) {
+			return {
+				toString: function toString() {
+					return input;
+				}
+			};
+		} );
+	}
+
+	startIframeTest( results );
+</script>
+</body>
+</html>
diff --git a/test/data/trusted-types-attributes.js b/test/data/trusted-types-attributes.js
@@ -0,0 +1 @@
+window.testMessage = "script run";
diff --git a/test/middleware-mockserver.js b/test/middleware-mockserver.js
@@ -264,6 +264,14 @@ var mocks = {
 		var body = fs.readFileSync( __dirname + "/data/trusted-html.html" ).toString();
 		resp.end( body );
 	},
+	trustedTypesAttributes: function( req, resp ) {
+		resp.writeHead( 200, {
+			"Content-Type": "text/html",
+			"Content-Security-Policy": "require-trusted-types-for 'script'; report-uri /base/test/data/mock.php?action=cspLog"
+		} );
+		var body = fs.readFileSync( __dirname + "/data/trusted-types-attributes.html" ).toString();
+		resp.end( body );
+	},
 	errorWithScript: function( req, resp ) {
 		if ( req.query.withScriptContentType ) {
 			resp.writeHead( 404, { "Content-Type": "application/javascript" } );
diff --git a/test/unit/attributes.js b/test/unit/attributes.js
@@ -1764,3 +1764,23 @@ QUnit.test( "non-lowercase boolean attribute getters should not crash", function
 		}
 	} );
 } );
+
+
+// Test trustedTypes support in browsers where they're supported (currently Chrome 83+).
+// Browsers with no TrustedScriptURL support still run tests on object wrappers with
+// a proper `toString` function.
+testIframe(
+	"Basic TrustedScriptURL support (gh-4948)",
+	"mock.php?action=trustedTypesAttributes",
+	function( assert, jQuery, window, document, test ) {
+		var done = assert.async();
+
+		assert.expect( 1 );
+
+		test.forEach( function( result ) {
+			assert.deepEqual( result.actual, result.expected, result.message );
+		} );
+
+		supportjQuery.get( baseURL + "mock.php?action=cspClean" ).then( done );
+	}
+);

Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ jQuery.extend( {`
`50`	`50`	`return ret;`
`51`	`51`	`}`
`52`	`52`
`53`		`- elem.setAttribute( name, value + "" );`
	`53`	`+ elem.setAttribute( name, value );`
`54`	`54`	`return value;`
`55`	`55`	`}`
`56`	`56`