Customize: Allow arbitrary custom CSS.

Update custom CSS validation to allow any CSS except `STYLE` close tags. Previously, some valid CSS would be rejected for containing HTML syntax characters, like this example:

{{{
@Property --animate {
  syntax: "<custom-ident>"; /* <-- Validation error on `<` */
  inherits: true;
  initial-value: false;
}
}}}

Developed in WordPress#10667.

Follow-up to [61418], [61486].

Props jonsurrell, westonruter, peterwilsoncc, johnbillion, xknown, sabernhardt, dmsnell, soyebsalar01, dlh.
Fixes #64418.


git-svn-id: https://develop.svn.wordpress.org/trunk@61527 602fd350-edb4-49c9-b593-d223f7449a82

Author: sirreal

src/wp-includes/customize/class-wp-customize-custom-css-setting.php

@@ -153,6 +153,11 @@ public function value() {
 	 * @since 4.7.0
 	 * @since 4.9.0 Checking for balanced characters has been moved client-side via linting in code editor.
 	 * @since 5.9.0 Renamed `$css` to `$value` for PHP 8 named parameter support.
+	 * @since 7.0.0 Only restricts contents which risk prematurely closing the STYLE element,
+	 *              either through a STYLE end tag or a prefix of one which might become a
+	 *              full end tag when combined with the contents of other styles.
+	 *
+	 * @see WP_REST_Global_Styles_Controller::validate_custom_css()
 	 *
 	 * @param string $value CSS to validate.
 	 * @return true|WP_Error True if the input was validated, otherwise WP_Error.
@@ -163,8 +168,73 @@ public function validate( $value ) {
 
 		$validity = new WP_Error();
 
-		if ( preg_match( '#</?\w+#', $css ) ) {
-			$validity->add( 'illegal_markup', __( 'Markup is not allowed in CSS.' ) );
+		$length = strlen( $css );
+		for (
+			$at = strcspn( $css, '<' );
+			$at < $length;
+			$at += strcspn( $css, '<', ++$at )
+		) {
+			$remaining_strlen = $length - $at;
+			/**
+			 * Custom CSS text is expected to render inside an HTML STYLE element.
+			 * A STYLE closing tag must not appear within the CSS text because it
+			 * would close the element prematurely.
+			 *
+			 * The text must also *not* end with a partial closing tag (e.g., `<`,
+			 * `</`, … `</style`) because subsequent styles which are concatenated
+			 * could complete it, forming a valid `</style>` tag.
+			 *
+			 * Example:
+			 *
+			 *     $style_a = 'p { font-weight: bold; </sty';
+			 *     $style_b = 'le> gotcha!';
+			 *     $combined = "{$style_a}{$style_b}";
+			 *
+			 *     $style_a = 'p { font-weight: bold; </style';
+			 *     $style_b = 'p > b { color: red; }';
+			 *     $combined = "{$style_a}\n{$style_b}";
+			 *
+			 * Note how in the second example, both of the style contents are benign
+			 * when analyzed on their own. The first style was likely the result of
+			 * improper truncation, while the second is perfectly sound. It was only
+			 * through concatenation that these two styles combined to form content
+			 * that would have broken out of the containing STYLE element, thus
+			 * corrupting the page and potentially introducing security issues.
+			 *
+			 * @see https://html.spec.whatwg.org/multipage/parsing.html#rawtext-end-tag-name-state
+			 */
+			$possible_style_close_tag = 0 === substr_compare(
+				$css,
+				'</style',
+				$at,
+				min( 7, $remaining_strlen ),
+				true
+			);
+			if ( $possible_style_close_tag ) {
+				if ( $remaining_strlen < 8 ) {
+					$validity->add(
+						'illegal_markup',
+						sprintf(
+							/* translators: %s is the CSS that was provided. */
+							__( 'The CSS must not end in "%s".' ),
+							esc_html( substr( $css, $at ) )
+						)
+					);
+					break;
+				}
+
+				if ( 1 === strspn( $css, " \t\f\r\n/>", $at + 7, 1 ) ) {
+					$validity->add(
+						'illegal_markup',
+						sprintf(
+							/* translators: %s is the CSS that was provided. */
+							__( 'The CSS must not contain "%s".' ),
+							esc_html( substr( $css, $at, 8 ) )
+						)
+					);
+					break;
+				}
+			}
 		}
 
 		if ( ! $validity->has_errors() ) {

src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php

@@ -674,6 +674,8 @@ public function get_theme_items( $request ) {
 	 *              either through a STYLE end tag or a prefix of one which might become a
 	 *              full end tag when combined with the contents of other styles.
 	 *
+	 * @see WP_Customize_Custom_CSS_Setting::validate()
+	 *
 	 * @param string $css CSS to validate.
 	 * @return true|WP_Error True if the input was validated, otherwise WP_Error.
 	 */
@@ -707,7 +709,7 @@ protected function validate_custom_css( $css ) {
 			 * Note how in the second example, both of the style contents are benign
 			 * when analyzed on their own. The first style was likely the result of
 			 * improper truncation, while the second is perfectly sound. It was only
-			 * through concatenation that these two scripts combined to form content
+			 * through concatenation that these two styles combined to form content
 			 * that would have broken out of the containing STYLE element, thus
 			 * corrupting the page and potentially introducing security issues.
 			 *

tests/phpunit/tests/customize/custom-css-setting.php

@@ -374,6 +374,31 @@ public function filter_update_custom_css_data( $data, $args ) {
 		return $data;
 	}
 
+	/**
+	 * Ensure that dangerous STYLE tag contents do not break HTML output.
+	 *
+	 * @ticket 64418
+	 * @covers ::wp_update_custom_css_post
+	 * @covers ::wp_custom_css_cb
+	 */
+	public function test_wp_custom_css_cb_escapes_dangerous_html() {
+		wp_update_custom_css_post(
+			'*::before { content: "</style><script>alert(1)</script>"; }',
+			array(
+				'stylesheet' => $this->setting->stylesheet,
+			)
+		);
+		$output   = get_echo( 'wp_custom_css_cb' );
+		$expected =
+			<<<'HTML'
+			<style id="wp-custom-css">
+			*::before { content: "\3c\2fstyle><script>alert(1)</script>"; }
+			</style>
+
+			HTML;
+		$this->assertEqualHTML( $expected, $output );
+	}
+
 	/**
 	 * Tests that validation errors are caught appropriately.
 	 *
@@ -382,8 +407,7 @@ public function filter_update_custom_css_data( $data, $args ) {
 	 *
 	 * @covers WP_Customize_Custom_CSS_Setting::validate
 	 */
-	public function test_validate() {
-
+	public function test_validate_basic_css() {
 		// Empty CSS throws no errors.
 		$result = $this->setting->validate( '' );
 		$this->assertTrue( $result );
@@ -393,9 +417,84 @@ public function test_validate() {
 		$result    = $this->setting->validate( $basic_css );
 		$this->assertTrue( $result );
 
-		// Check for markup.
+		// Check for illegal closing STYLE tag.
 		$unclosed_comment = $basic_css . '</style>';
 		$result           = $this->setting->validate( $unclosed_comment );
 		$this->assertArrayHasKey( 'illegal_markup', $result->errors );
 	}
+
+	/**
+	 * @ticket 64418
+	 * @covers WP_Customize_Custom_CSS_Setting::validate
+	 */
+	public function test_validate_accepts_css_property_at_rule() {
+		$css =
+			<<<'CSS'
+			@property --animate {
+				syntax: "<custom-ident>";
+				inherits: true;
+				initial-value: false;
+			}
+			CSS;
+		$this->assertTrue( $this->setting->validate( $css ) );
+	}
+
+	/**
+	 * @ticket 64418
+	 * @covers ::wp_update_custom_css_post
+	 * @covers ::wp_custom_css_cb
+	 */
+	public function test_save_and_print_property_at_rule() {
+		$css =
+			<<<'CSS'
+			@property --animate {
+				syntax: "<custom-ident>";
+				inherits: true;
+				initial-value: false;
+			}
+			CSS;
+		wp_update_custom_css_post( $css, array( 'stylesheet' => $this->setting->stylesheet ) );
+		$output   = get_echo( 'wp_custom_css_cb' );
+		$expected = "<style id='wp-custom-css'>\n{$css}\n</style>\n";
+		$this->assertEqualHTML( $expected, $output );
+	}
+
+	/**
+	 * @dataProvider data_custom_css_disallowed
+	 *
+	 * @ticket 64418
+	 * @covers WP_Customize_Custom_CSS_Setting::validate
+	 */
+	public function test_validate_prevents( $css, $expected_error_message ) {
+		$result = $this->setting->validate( $css );
+		$this->assertWPError( $result );
+		$this->assertSame( $expected_error_message, $result->get_error_message() );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array<string, string[]>
+	 */
+	public static function data_custom_css_disallowed(): array {
+		return array(
+			'style close tag'            => array( 'css…</style>…css', 'The CSS must not contain "&lt;/style&gt;".' ),
+			'style close tag upper case' => array( '</STYLE>', 'The CSS must not contain "&lt;/STYLE&gt;".' ),
+			'style close tag mixed case' => array( '</sTyLe>', 'The CSS must not contain "&lt;/sTyLe&gt;".' ),
+			'style close tag in comment' => array( '/*</style>*/', 'The CSS must not contain "&lt;/style&gt;".' ),
+			'style close tag (/)'        => array( '</style/', 'The CSS must not contain "&lt;/style/".' ),
+			'style close tag (\t)'       => array( "</style\t", "The CSS must not contain \"&lt;/style\t\"." ),
+			'style close tag (\f)'       => array( "</style\f", "The CSS must not contain \"&lt;/style\f\"." ),
+			'style close tag (\r)'       => array( "</style\r", "The CSS must not contain \"&lt;/style\r\"." ),
+			'style close tag (\n)'       => array( "</style\n", "The CSS must not contain \"&lt;/style\n\"." ),
+			'style close tag (" ")'      => array( '</style ', 'The CSS must not contain "&lt;/style ".' ),
+			'truncated "<"'              => array( '<', 'The CSS must not end in "&lt;".' ),
+			'truncated "</"'             => array( '</', 'The CSS must not end in "&lt;/".' ),
+			'truncated "</s"'            => array( '</s', 'The CSS must not end in "&lt;/s".' ),
+			'truncated "</ST"'           => array( '</ST', 'The CSS must not end in "&lt;/ST".' ),
+			'truncated "</sty"'          => array( '</sty', 'The CSS must not end in "&lt;/sty".' ),
+			'truncated "</STYL"'         => array( '</STYL', 'The CSS must not end in "&lt;/STYL".' ),
+			'truncated "</stYle"'        => array( '</stYle', 'The CSS must not end in "&lt;/stYle".' ),
+		);
+	}
 }