Changeset 61527

Timestamp:

01/26/2026 03:17:00 PM (3 weeks ago)

Author:

jonsurrell

Message:

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 ​https://github.com/WordPress/wordpress-develop/pull/10667.

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

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

Location:

trunk

Files:

• src/wp-includes/customize/class-wp-customize-custom-css-setting.php (modified) (2 diffs)
• src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php (modified) (2 diffs)
• tests/phpunit/tests/customize/custom-css-setting.php (modified) (3 diffs)

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

@@ -154 +154 @@
      * @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.
@@ -164 +169 @@
         $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;
+                }
+            }
         }
 

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

@@ -674 +674 @@
      *              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.
@@ -708 +710 @@
              * 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.

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

@@ -376 +376 @@
 
     /**
+     * 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.
      *
@@ -383 +408 @@
      * @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( '' );
@@ -394 +418 @@
         $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".' ),
+        );
+    }
 }