Changeset 61486

Timestamp:

01/15/2026 12:08:01 PM (5 weeks 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".' ),
+        );
+    }
+}