diff --git a/src/wp-includes/kses.php b/src/wp-includes/kses.php index c68b71fcc1527..f012801f33663 100644 --- a/src/wp-includes/kses.php +++ b/src/wp-includes/kses.php @@ -1874,10 +1874,7 @@ public function sanitize() { } } - $is_url_ish = in_array( strtolower( $name ), $this->uri_attributes, true ); - if ( $is_url_ish ) { - $value = wp_kses_bad_protocol( $value, $this->allowed_protocols ); - } + $value = _wp_kses_sanitize_url_attribute( $tag_name, strtolower( $name ), $value, $this->allowed_protocols, $this->uri_attributes ); /* * Process the remaining attributes according to their policies. @@ -2093,9 +2090,14 @@ function wp_kses_one_attr( $attr, $element ) { // Sanitize quotes, angle braces, and entities. $value = esc_attr( $value ); - // Sanitize URI values. - if ( in_array( strtolower( $name ), $uris, true ) ) { - $value = wp_kses_bad_protocol( $value, $allowed_protocols ); + /* + * Sanitize URL values. The value is already escaped, so it is decoded + * for sanitization and re-encoded only when sanitization changed it. + */ + $decoded = WP_HTML_Decoder::decode_attribute( $value ); + $sanitized = _wp_kses_sanitize_url_attribute( strtolower( $element ), strtolower( $name ), $decoded, $allowed_protocols, $uris ); + if ( $sanitized !== $decoded ) { + $value = _wp_specialchars( $sanitized, ENT_QUOTES, false, true ); } $attr = "$name=$quote$value$quote"; @@ -2415,6 +2417,50 @@ function wp_kses_uri_attributes() { return $uri_attributes; } +/** + * Sanitizes the value of an attribute that a browser reads as a URL or a list of URLs. + * + * Most URL attributes hold a single URL and are listed by {@see wp_kses_uri_attributes()}. + * A few hold a list whose syntax depends on the element they appear on: `srcset` on + * `img` and `source`, and `imagesrcset` on `link`, hold a comma-separated list of + * image candidate strings and are handled by {@see wp_kses_sanitize_srcset()}. On any + * other element a browser ignores those attributes, so they are left alone. + * + * Takes and returns decoded attribute text. + * + * @since 7.2.0 + * @access private + * + * @param string $element Lowercase element name, or an empty string when unknown. + * @param string $attribute Lowercase attribute name. + * @param string $value Decoded attribute value. + * @param string[] $allowed_protocols Array of allowed URL protocols. + * @param string[] $uri_attributes Names of attributes holding a single URL, + * as returned by wp_kses_uri_attributes(). + * @return string Sanitized value. + */ +function _wp_kses_sanitize_url_attribute( string $element, string $attribute, string $value, array $allowed_protocols, array $uri_attributes ): string { + switch ( $attribute ) { + case 'srcset': + if ( '' === $element || 'img' === $element || 'source' === $element ) { + return wp_kses_sanitize_srcset( $value, $allowed_protocols ); + } + break; + + case 'imagesrcset': + if ( '' === $element || 'link' === $element ) { + return wp_kses_sanitize_srcset( $value, $allowed_protocols ); + } + break; + } + + if ( in_array( $attribute, $uri_attributes, true ) ) { + return wp_kses_bad_protocol( $value, $allowed_protocols ); + } + + return $value; +} + /** * Callback for `wp_kses_split()`. * @@ -2604,7 +2650,7 @@ function wp_kses_attr( $element, $attr, $allowed_html, $allowed_protocols ) { } // Split it. - $attrarr = wp_kses_hair( $attr, $allowed_protocols ); + $attrarr = wp_kses_hair( $attr, $allowed_protocols, $element ); // Check if there are attributes that are required. $required_attrs = array_filter( @@ -2746,7 +2792,7 @@ function wp_kses_attr_check( &$name, &$value, &$whole, $vless, $element, $allowe * This function performs a number of transformations while parsing attribute strings: * - It normalizes attribute values and surrounds them with double quotes. * - It normalizes HTML character references inside attribute values. - * - It removes “bad” URL protocols from attribute values. + * - It removes “bad” URL protocols from attribute values, including each URL in a `srcset`. * * Otherwise this reads the attributes as if they were part of an HTML tag. It performs * these transformations to lower the risk of mis-parsing down the line and to perform @@ -2767,14 +2813,18 @@ function wp_kses_attr_check( &$name, &$value, &$whole, $vless, $element, $allowe * * @since 1.0.0 * @since 7.0.0 Reliably parses HTML via the HTML API. + * @since 7.2.0 Added the optional `$element` parameter. * * @param string $attr Attribute list from HTML element to closing HTML element tag. * @param string[] $allowed_protocols Array of allowed URL protocols. + * @param string $element Optional. Name of the element the attributes belong to, which + * decides how URL attributes such as `srcset` are read. Default ''. * @return array Array of attribute information after parsing. */ -function wp_kses_hair( $attr, $allowed_protocols ) { +function wp_kses_hair( $attr, $allowed_protocols, $element = '' ) { $attributes = array(); $uris = wp_kses_uri_attributes(); + $element = strtolower( (string) $element ); $processor = new WP_HTML_Tag_Processor( "" ); $processor->next_token(); @@ -2795,8 +2845,8 @@ function wp_kses_hair( $attr, $allowed_protocols ) { foreach ( $attribute_names as $name ) { $value = $processor->get_attribute( $name ); $is_bool = true === $value; - if ( is_string( $value ) && in_array( $name, $uris, true ) ) { - $value = wp_kses_bad_protocol( $value, $allowed_protocols ); + if ( is_string( $value ) ) { + $value = _wp_kses_sanitize_url_attribute( $element, $name, $value, $allowed_protocols, $uris ); } // Reconstruct and normalize the attribute value. @@ -3200,6 +3250,97 @@ function wp_kses_bad_protocol_once2( $scheme, $allowed_protocols ) { } } +/** + * Removes image candidates with a disallowed URL protocol from a `srcset` value. + * + * A `srcset` attribute holds a comma-separated list of image candidate strings, + * each a URL optionally followed by whitespace and width or density descriptors, + * for example `small.jpg 480w, large.jpg 1024w`. The value is split into candidates + * following the HTML specification's srcset parsing algorithm, so a comma only + * separates candidates when it ends a URL or its descriptors; a comma inside a URL, + * as produced by image CDN resizers, belongs to the URL. + * + * Each URL is checked with {@see wp_kses_bad_protocol()}. A candidate whose URL that + * check would change is removed in full, descriptors included, because stripping a + * scheme and keeping the rest would rewrite the URL rather than remove it, and + * descriptors left behind would be read as the next candidate's URL. As in + * {@see esc_url()}, a URL starting with `/` cannot carry a scheme and is kept as is. + * + * The value must be decoded attribute text, as returned by + * {@see WP_HTML_Tag_Processor::get_attribute()}, not raw HTML containing character + * references. The returned value is decoded text too; callers writing it back into + * HTML must escape it. When every candidate is allowed, the input is returned + * unchanged. Otherwise the remaining candidates are rejoined with `, `. + * + * @since 7.2.0 + * + * @link https://html.spec.whatwg.org/multipage/images.html#parsing-a-srcset-attribute + * + * @param string $srcset Decoded `srcset` attribute value. + * @param string[] $allowed_protocols Array of allowed URL protocols. + * @return string The value without candidates whose URL uses a disallowed protocol. + */ +function wp_kses_sanitize_srcset( string $srcset, array $allowed_protocols ): string { + $whitespace = " \t\f\r\n"; + $length = strlen( $srcset ); + $at = 0; + $result = ''; + + /* + * Candidates are only copied once one has to be removed. Until then the + * scan allocates nothing but the URLs it checks, and an all-allowed value + * is returned as the very string that was passed in. + * + * On the first removal the scan restarts to rebuild the output; candidates + * before `$checked_until` passed their check in the first pass and are not + * checked again. + */ + $rebuild = false; + $checked_until = 0; + + while ( $at < $length ) { + $at += strspn( $srcset, "{$whitespace},", $at ); + if ( $at >= $length ) { + break; + } + + // The URL is the run of non-whitespace characters, less any commas ending it. + $candidate_at = $at; + $url_length = strcspn( $srcset, $whitespace, $at ); + $url = rtrim( substr( $srcset, $at, $url_length ), ',' ); + $at += $url_length; + + // A URL ended by a comma has no descriptors; otherwise they run to the next comma. + if ( strlen( $url ) === $url_length ) { + $at += strcspn( $srcset, ',', $at ); + $candidate_length = $at - $candidate_at; + } else { + $candidate_length = strlen( $url ); + } + + if ( $rebuild && $candidate_at < $checked_until ) { + $is_allowed = true; + } else { + $is_allowed = '/' === $url[0] || 0 === strcasecmp( wp_kses_bad_protocol( $url, $allowed_protocols ), $url ); + } + + if ( $rebuild ) { + if ( $is_allowed ) { + $result .= ( '' === $result ? '' : ', ' ) . rtrim( substr( $srcset, $candidate_at, $candidate_length ), $whitespace ); + } + continue; + } + + if ( ! $is_allowed ) { + $rebuild = true; + $checked_until = $candidate_at; + $at = 0; + } + } + + return $rebuild ? $result : $srcset; +} + /** * Converts and fixes HTML entities. * diff --git a/tests/phpunit/tests/kses.php b/tests/phpunit/tests/kses.php index 74a136569ad66..5d43b2d912597 100644 --- a/tests/phpunit/tests/kses.php +++ b/tests/phpunit/tests/kses.php @@ -2995,4 +2995,140 @@ public function data_allowed_attributes_in_descriptions() { ), ); } + + /** + * @ticket 29807 + * + * @dataProvider data_srcset_is_sanitized_by_element + * + * @param bool $legacy_parser Whether to run the legacy parser instead of the HTML API one. + * @param string $html Input HTML. + * @param string $expected Expected output. + */ + public function test_srcset_is_sanitized_by_element( bool $legacy_parser, string $html, string $expected ) { + if ( $legacy_parser ) { + add_filter( 'wp_kses_force_legacy_parser', '__return_true' ); + } + + $allowed_html = array( + 'img' => array( + 'src' => true, + 'srcset' => true, + ), + 'source' => array( 'srcset' => true ), + 'link' => array( + 'rel' => true, + 'imagesrcset' => true, + ), + 'span' => array( 'srcset' => true ), + ); + + $this->assertEqualHTML( $expected, wp_kses( $html, $allowed_html ) ); + } + + /** + * Returns each case once per parser. + * + * @return array + */ + public function data_srcset_is_sanitized_by_element() { + $cases = array( + 'allowed candidates are kept' => array( + '', + '', + ), + 'disallowed candidate is removed from img' => array( + '', + '', + ), + 'every candidate disallowed empties srcset' => array( + '', + '', + ), + 'character references are decoded first' => array( + '', + '', + ), + 'source srcset is sanitized' => array( + '', + '', + ), + 'link imagesrcset is sanitized' => array( + '', + '', + ), + // Browsers do not read srcset on other elements, so the value is not parsed as one. + 'srcset on another element is left alone' => array( + 'text', + 'text', + ), + 'src is still sanitized as a single URL' => array( + '', + '', + ), + ); + + $data = array(); + foreach ( $cases as $name => $case ) { + $data[ "{$name} (HTML API parser)" ] = array( false, ...$case ); + $data[ "{$name} (legacy parser)" ] = array( true, ...$case ); + } + + return $data; + } + + /** + * @ticket 29807 + * + * @dataProvider data_one_attr_sanitizes_srcset + * + * @param string $element Element the attribute belongs to. + * @param string $input Attribute as written. + * @param string $expected Filtered attribute. + */ + public function test_one_attr_sanitizes_srcset( string $element, string $input, string $expected ) { + add_filter( + 'wp_kses_allowed_html', + static function ( $tags ) { + $tags['img']['srcset'] = true; + $tags['span']['srcset'] = true; + return $tags; + } + ); + + $this->assertSame( $expected, wp_kses_one_attr( $input, $element ) ); + } + + /** + * @return array + */ + public function data_one_attr_sanitizes_srcset() { + return array( + 'allowed candidates are kept' => array( + 'img', + 'srcset="a.jpg 1x, b.jpg 2x"', + 'srcset="a.jpg 1x, b.jpg 2x"', + ), + 'disallowed candidate is removed' => array( + 'img', + 'srcset="a.jpg 1x, javascript:alert(1) 2x"', + 'srcset="a.jpg 1x"', + ), + 'character references are decoded first' => array( + 'img', + 'srcset="a.jpg 1x, javascript:alert(1) 2x"', + 'srcset="a.jpg 1x"', + ), + 'ampersand in a kept URL stays escaped' => array( + 'img', + 'srcset="a.jpg?x=1&y=2 1x, javascript:alert(1) 2x"', + 'srcset="a.jpg?x=1&y=2 1x"', + ), + 'srcset on another element is left alone' => array( + 'span', + 'srcset="javascript:alert(1) 1x"', + 'srcset="javascript:alert(1) 1x"', + ), + ); + } } diff --git a/tests/phpunit/tests/kses/wpKsesHair.php b/tests/phpunit/tests/kses/wpKsesHair.php index 0ef834880cd5c..8462783f9913a 100644 --- a/tests/phpunit/tests/kses/wpKsesHair.php +++ b/tests/phpunit/tests/kses/wpKsesHair.php @@ -1110,4 +1110,43 @@ public function test_custom_allowed_protocols() { $this->assertSame( $expected, $result ); } + + /** + * @ticket 29807 + * + * @dataProvider data_srcset_is_sanitized_for_image_elements + * @covers wp_kses_hair + * + * @param string $element Element passed to wp_kses_hair(). + * @param string $expected Expected srcset value. + */ + public function test_srcset_is_sanitized_for_image_elements( string $element, string $expected ) { + $result = wp_kses_hair( 'srcset="a.jpg 1x, javascript:alert(1) 2x"', $this->allowed_protocols, $element ); + + $this->assertSame( $expected, $result['srcset']['value'] ); + $this->assertSame( "srcset=\"{$expected}\"", $result['srcset']['whole'] ); + } + + /** + * @return array + */ + public function data_srcset_is_sanitized_for_image_elements() { + return array( + 'img' => array( 'img', 'a.jpg 1x' ), + 'source' => array( 'source', 'a.jpg 1x' ), + 'uppercase element' => array( 'IMG', 'a.jpg 1x' ), + 'unknown element' => array( '', 'a.jpg 1x' ), + 'other element' => array( 'span', 'a.jpg 1x, javascript:alert(1) 2x' ), + ); + } + + /** + * @ticket 29807 + * @covers wp_kses_hair + */ + public function test_single_url_attributes_are_sanitized_regardless_of_element() { + $result = wp_kses_hair( 'href="javascript:alert(1)"', $this->allowed_protocols, 'span' ); + + $this->assertSame( 'alert(1)', $result['href']['value'] ); + } } diff --git a/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php new file mode 100644 index 0000000000000..99679fd36fc13 --- /dev/null +++ b/tests/phpunit/tests/kses/wpKsesSanitizeSrcset.php @@ -0,0 +1,153 @@ +allowed_protocols = wp_allowed_protocols(); + } + + /** + * @ticket 29807 + * + * @dataProvider data_allowed_values_are_returned_unchanged + * + * @param string $srcset Value whose candidates are all allowed. + */ + public function test_allowed_values_are_returned_unchanged( string $srcset ) { + $this->assertSame( $srcset, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); + } + + /** + * @return array + */ + public function data_allowed_values_are_returned_unchanged() { + return array( + 'empty value' => array( '' ), + 'whitespace only' => array( " \t\n" ), + 'single URL, no descriptor' => array( 'image.jpg' ), + 'relative URLs' => array( 'path/to/test.png 1x, path/to/test-2x.png 2x' ), + 'root-relative URLs' => array( '/test.png 1x, /test-2x.png 2x' ), + 'protocol-relative URLs' => array( '//cdn.example.com/img-300.jpg 300w, //cdn.example.com/img-600.jpg 600w' ), + 'absolute URLs with width descriptors' => array( 'https://example.com/small.jpg 480w, https://example.com/medium.jpg 800w, https://example.com/large.jpg 1200w' ), + 'uppercase scheme' => array( 'HTTPS://example.com/a.jpg 1x, Http://example.com/b.jpg 2x' ), + 'decimal density descriptors' => array( 'image-1x.jpg 1x, image-1.5x.jpg 1.5x, image-2x.jpg 2x' ), + 'no descriptor on the last candidate' => array( 'small.jpg 300w, medium.jpg 600w, large.jpg' ), + 'no whitespace after the comma' => array( 'image1.jpg 1x,image2.jpg 2x' ), + 'extra whitespace is preserved' => array( ' image1.jpg 1x , image2.jpg 2x ' ), + 'newlines are preserved' => array( "image1.jpg 1x,\nimage2.jpg 2x" ), + 'commas inside a URL belong to the URL' => array( 'http://localhost/test.png,big 1x, http://localhost/test.png,medium 2x' ), + 'CDN resizer URLs with commas' => array( 'https://resizer.example/cdn-cgi/image/format=auto,onerror=redirect,quality=80,width=412,height=275,dpr=1,fit=crop,gravity=0.5x0.5/https://bucket.example/photo.jpg 412w, https://resizer.example/cdn-cgi/image/format=auto,quality=80,width=824,height=550/https://bucket.example/photo.jpg 824w' ), + 'CDN resizer URL without descriptor' => array( 'https://cdn.example/image/format=auto,quality=80/img.jpg 400w, https://cdn.example/image/format=auto,quality=80/img-large.jpg' ), + 'root-relative URL containing a colon' => array( '/images/a:b.jpg 1x' ), + ); + } + + /** + * @ticket 29807 + * + * @dataProvider data_candidates_with_disallowed_protocols_are_removed + * + * @param string $srcset Value containing at least one disallowed candidate. + * @param string $expected Value with those candidates removed. + */ + public function test_candidates_with_disallowed_protocols_are_removed( string $srcset, string $expected ) { + $this->assertSame( $expected, wp_kses_sanitize_srcset( $srcset, $this->allowed_protocols ) ); + } + + /** + * @return array + */ + public function data_candidates_with_disallowed_protocols_are_removed() { + return array( + 'first candidate' => array( + 'bad://localhost/test.png 1x, http://localhost/test-2x.png 2x', + 'http://localhost/test-2x.png 2x', + ), + 'last candidate' => array( + 'http://localhost/test.png 1x, bad://localhost/test-2x.png 2x', + 'http://localhost/test.png 1x', + ), + 'middle candidate' => array( + 'image-1x.jpg 1x, javascript:alert(1) 1.5x, image-2x.jpg 2x', + 'image-1x.jpg 1x, image-2x.jpg 2x', + ), + 'only candidate' => array( + 'javascript:alert(1) 1x', + '', + ), + 'every candidate' => array( + 'javascript:alert(1) 1x, data:text/html, 2x', + '', + ), + 'two in a row' => array( + 'a.jpg 1x, javascript:alert(1) 2x, vbscript:msgbox(1) 3x, d.jpg 4x', + 'a.jpg 1x, d.jpg 4x', + ), + 'data URL' => array( + 'data:image/png;base64,iVBORw0KGgo= 1x, https://example.com/a.jpg 2x', + 'https://example.com/a.jpg 2x', + ), + 'URL with commas and a disallowed scheme' => array( + 'http://localhost/test.png,big 1x, bad://localhost/test.png,medium 2x', + 'http://localhost/test.png,big 1x', + ), + 'descriptor-less candidate after a comma' => array( + 'safe.jpg, javascript:alert(1)', + 'safe.jpg', + ), + 'comma-joined URLs form one candidate' => array( + 'safe.jpg,javascript:alert(1) 2x', + '', + ), + 'comma attached to an invalid descriptor' => array( + 'a.jpg 2q,javascript:alert(1) 2x', + 'a.jpg 2q', + ), + 'descriptors are not sanitized as URLs' => array( + 'a.jpg javascript:alert(1), b.jpg 2x', + 'a.jpg javascript:alert(1), b.jpg 2x', + ), + 'scheme that empties under wp_kses_bad_protocol' => array( + 'a:b:c:d:e:f:g 100w, safe.jpg 200w', + 'safe.jpg 200w', + ), + 'emptied scheme without descriptor' => array( + 'a:b:c:d:e:f:g, safe.jpg 200w', + 'safe.jpg 200w', + ), + 'separators are normalized once a candidate is removed' => array( + " a.jpg 1x ,\n\tjavascript:alert(1) 2x ,b.jpg 3x ", + 'a.jpg 1x, b.jpg 3x', + ), + 'allowed scheme is kept in its original case' => array( + 'HTTPS://example.com/a.jpg 1x, javascript:alert(1) 2x', + 'HTTPS://example.com/a.jpg 1x', + ), + ); + } + + /** + * @ticket 29807 + */ + public function test_respects_the_given_allowed_protocols() { + $srcset = 'https://example.com/a.jpg 1x, ftp://example.com/b.jpg 2x'; + + $this->assertSame( $srcset, wp_kses_sanitize_srcset( $srcset, array( 'https', 'ftp' ) ) ); + $this->assertSame( 'https://example.com/a.jpg 1x', wp_kses_sanitize_srcset( $srcset, array( 'https' ) ) ); + $this->assertSame( '', wp_kses_sanitize_srcset( $srcset, array() ) ); + } +}