Global Styles: Allow arbitrary CSS, protect from KSES mangling

src/wp-includes/kses.php

@@ -2386,7 +2386,13 @@ function wp_filter_global_styles_post( $data ) {
 		$data_to_encode = WP_Theme_JSON::remove_insecure_properties( $decoded_data, 'custom' );
 
 		$data_to_encode['isGlobalStylesUserThemeJSON'] = true;
-		return wp_slash( wp_json_encode( $data_to_encode ) );
+		/**
+		 * JSON encode the data stored in post content.
+		 * Escape characters that are likely to be mangled by HTML filters: "<>&".
+		 *
+		 * This matches the escaping in {@see WP_REST_Global_Styles_Controller::prepare_item_for_database()}.
+		 */
+		return wp_slash( wp_json_encode( $data_to_encode, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP ) );
 	}
 	return $data;
 }

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

@@ -275,7 +275,14 @@ protected function prepare_item_for_database( $request ) {
 			}
 			$config['isGlobalStylesUserThemeJSON'] = true;
 			$config['version']                     = WP_Theme_JSON::LATEST_SCHEMA;
-			$changes->post_content                 = wp_json_encode( $config );
+			/**
+			 * JSON encode the data stored in post content.
+			 * Escape characters that are likely to be mangled by HTML filters: "<>&".
+			 *
+			 * This data is later re-encoded by {@see wp_filter_global_styles_post()}.
+			 * The escaping is also applied here as a precaution.
+			 */
+			$changes->post_content = wp_json_encode( $config, JSON_UNESCAPED_SLASHES | JSON_HEX_TAG | JSON_HEX_AMP );
 		}
 
 		// Post title.
@@ -659,22 +666,87 @@ public function get_theme_items( $request ) {
 	/**
 	 * Validate style.css as valid CSS.
 	 *
-	 * Currently just checks for invalid markup.
+	 * Currently just checks that CSS will not break an HTML STYLE tag.
 	 *
 	 * @since 6.2.0
 	 * @since 6.4.0 Changed method visibility to protected.
+	 * @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.
 	 *
 	 * @param string $css CSS to validate.
 	 * @return true|WP_Error True if the input was validated, otherwise WP_Error.
 	 */
 	protected function validate_custom_css( $css ) {
-		if ( preg_match( '#</?\w+#', $css ) ) {
-			return new WP_Error(
-				'rest_custom_css_illegal_markup',
-				__( 'Markup is not allowed in CSS.' ),
-				array( 'status' => 400 )
+		$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 scripts 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 ) {
+					return new WP_Error(
+						'rest_custom_css_illegal_markup',
+						sprintf(
+							/* translators: %s is the CSS that was provided. */
+							__( 'The CSS must not end in "%s".' ),
+							esc_html( substr( $css, $at ) )
+						),
+						array( 'status' => 400 )
+					);
+				}
+
+				if ( 1 === strspn( $css, " \t\f\r\n/>", $at + 7, 1 ) ) {
+					return new WP_Error(
+						'rest_custom_css_illegal_markup',
+						sprintf(
+							/* translators: %s is the CSS that was provided. */
+							__( 'The CSS must not contain "%s".' ),
+							esc_html( substr( $css, $at, 8 ) )
+						),
+						array( 'status' => 400 )
+					);
+				}
+			}
 		}
+
 		return true;
 	}
 }

tests/phpunit/tests/rest-api/rest-global-styles-controller.php

@@ -650,6 +650,7 @@ public function test_update_item_valid_styles_css() {
 	/**
 	 * @covers WP_REST_Global_Styles_Controller::update_item
 	 * @ticket 57536
+	 * @ticket 64418
 	 */
 	public function test_update_item_invalid_styles_css() {
 		wp_set_current_user( self::$admin_id );
@@ -659,7 +660,7 @@ public function test_update_item_invalid_styles_css() {
 		$request = new WP_REST_Request( 'PUT', '/wp/v2/global-styles/' . self::$global_styles_id );
 		$request->set_body_params(
 			array(
-				'styles' => array( 'css' => '<p>test</p> body { color: red; }' ),
+				'styles' => array( 'css' => '</style>' ),
 			)
 		);
 		$response = rest_get_server()->dispatch( $request );
@@ -826,4 +827,123 @@ public function test_global_styles_route_args_schema() {
 		$this->assertArrayHasKey( 'type', $route_data[0]['args']['id'] );
 		$this->assertSame( 'integer', $route_data[0]['args']['id']['type'] );
 	}
+
+	/**
+	 * @covers WP_REST_Global_Styles_Controller::update_item
+	 * @ticket 64418
+	 */
+	public function test_update_allows_valid_css_with_more_syntax() {
+		wp_set_current_user( self::$admin_id );
+		if ( is_multisite() ) {
+			grant_super_admin( self::$admin_id );
+		}
+		$request = new WP_REST_Request( 'PUT', '/wp/v2/global-styles/' . self::$global_styles_id );
+		$css     = <<<'CSS'
+@property --animate {
+	syntax: "<custom-ident>";
+	inherits: true;
+	initial-value: false;
+}
+h1::before { content: "fun & games"; }
+CSS;
+		$request->set_body_params(
+			array(
+				'styles' => array( 'css' => $css ),
+			)
+		);
+
+		$response = rest_get_server()->dispatch( $request );
+		$data     = $response->get_data();
+		$this->assertSame( $css, $data['styles']['css'] );
+
+		// Compare expected API output to WP internal values.
+		$request  = new WP_REST_Request( 'GET', '/wp/v2/global-styles/' . self::$global_styles_id );
+		$response = rest_get_server()->dispatch( $request );
+		$this->assertSame( $css, $response->get_data()['styles']['css'] );
+	}
+
+	/**
+	 * @covers WP_REST_Global_Styles_Controller::validate_custom_css
+	 * @ticket 64418
+	 *
+	 * @dataProvider data_custom_css_allowed
+	 */
+	public function test_validate_custom_css_allowed( string $custom_css ) {
+		$controller = new WP_REST_Global_Styles_Controller();
+		$validate   = Closure::bind(
+			function ( $css ) {
+				return $this->validate_custom_css( $css );
+			},
+			$controller,
+			$controller
+		);
+
+		$this->assertTrue( $validate( $custom_css ) );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array<string, string[]>
+	 */
+	public static function data_custom_css_allowed(): array {
+		return array(
+			'@property declaration'   => array(
+				'@property --prop { syntax: "<custom-ident>"; inherits: true; initial-value: false; }',
+			),
+			'Different close tag'     => array( '</stylesheet>' ),
+			'Not a style close tag'   => array( '/*</style*/' ),
+			'Not a style close tag 2' => array( '/*</style_' ),
+			'Empty'                   => array( '' ),
+			'Short content'           => array( '/**/' ),
+		);
+	}
+
+	/**
+	 * @covers WP_REST_Global_Styles_Controller::validate_custom_css
+	 * @ticket 64418
+	 *
+	 * @dataProvider data_custom_css_disallowed
+	 */
+	public function test_validate_custom_css( string $custom_css, string $expected_error_message ) {
+		$controller = new WP_REST_Global_Styles_Controller();
+		$validate   = Closure::bind(
+			function ( $css ) {
+				return $this->validate_custom_css( $css );
+			},
+			$controller,
+			$controller
+		);
+
+		$result = $validate( $custom_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".' ),
+		);
+	}
 }