Changeset 61486

Timestamp:

01/15/2026 12:08:01 PM (7 months ago)

Author:

jonsurrell

Message:

Customize: Allow arbitrary CSS in global styles custom CSS.

Relax Global Styles custom CSS filters to allow arbitrary CSS.

Escape HTML characters <>& in Global Styles data to prevent it from being mangled by post content filters. The data is JSON encoded and stored in post_content. Filters operating on post_content expect it to contain HTML. Some KSES filters would otherwise remove essential CSS features like the <custom-ident> CSS data type because they appear to be HTML tags.

[61418] changed STYLE tag generation to use the HTML API for improved safety.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10641.

Props jonsurrell, dmsnell, westonruter, ramonopoly, oandregal, jorgefilipecosta, sabernhardt, soyebsalar01.
See #64418.

Location:

trunk

Files:

• src/wp-includes/kses.php (modified) (1 diff)
• src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php (modified) (3 diffs)
• tests/phpunit/tests/rest-api/rest-global-styles-controller.php (modified) (3 diffs)

trunk/src/wp-includes/kses.php

@@ -2387 +2387 @@
 
                 $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;

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

@@ -276 +276 @@
                         $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 );
                 }
 
@@ -660 +667 @@
          * 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.
@@ -669 +679 @@
          */
         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;
         }

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

@@ -651 +651 @@
          * @covers WP_REST_Global_Styles_Controller::update_item
          * @ticket 57536
+         * @ticket 64418
          */
         public function test_update_item_invalid_styles_css() {
@@ -660 +661 @@
                 $request->set_body_params(
                         array(
-                                'styles' => array( 'css' => '<p>test</p> body { color: red; }' ),
+                                'styles' => array( 'css' => '</style>' ),
                         )
                 );
@@ -827 +828 @@
                 $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".' ),
+                );
+        }
+}