Make WordPress Core

Opened 17 months ago

Last modified 7 weeks ago

#62224 new enhancement

The class class-wp-theme-json.php methods compute_theme_vars and to_ruleset need to be hardened

Reported by: villu164's profile villu164 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 6.6.2
Component: Themes Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

I first reported the issue on HackerOne and was told this can be a public hardening ticket. (https://hackerone.com/bugs?subject=user&report_id=2623479)

The compute_theme_vars (https://core.trac.wordpress.org/browser/tags/6.6.2/src/wp-includes/class-wp-theme-json.php#L2208) and to_ruleset (https://core.trac.wordpress.org/browser/tags/6.6.2/src/wp-includes/class-wp-theme-json.php#L1900) need to be hardened and the theme.json, should be considered as user supplied content. Thus the before-mentioned methods need to adjust for that and use proper sanitization

Change History (5)

This ticket was mentioned in PR #10715 on WordPress/wordpress-develop by @jaysinh146.


8 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Adds comprehensive sanitization to WP_Theme_JSON::compute_theme_vars() and WP_Theme_JSON::to_ruleset() to treat theme.json as user-supplied content.

Security improvements:

  • Sanitizes CSS variable names (alphanumeric + hyphens only)
  • Sanitizes CSS selectors to prevent selector injection
  • Sanitizes CSS property names and values
  • Quote-aware parsing preserves legitimate CSS syntax
  • Blocks CSS structure characters (;, {, }) outside quotes
  • Blocks dangerous URL protocols (javascript:, data:, vbscript:)
  • Blocks CSS at-rules (@import, @charset, @namespace)
  • Blocks legacy browser attacks (expression, behavior, -moz-binding)
  • Enforces length limits to prevent DoS attacks

New sanitization methods in WP_Theme_JSON:

  • sanitize_css_selector() - Validates CSS selectors
  • sanitize_css_property_name() - Validates property names
  • sanitize_css_property_value() - Validates property values

Test coverage:

  • tests/phpunit/tests/theme/wpThemeJsonComputeThemeVars.php (23 tests)
  • tests/phpunit/tests/theme/wpThemeJsonToRuleset.php (27 tests)
  • Total: 50 test methods, all passing

Props: villu164
Fixes #62224

Trac ticket: https://core.trac.wordpress.org/ticket/62224

@jaysinh146 commented on PR #10715:


8 weeks ago
#2

@mukeshpanchal27 Thank you for the review! I've updated the @since tag to 7.0.0 as requested.

## Regarding the 174 PHPUnit Test "Failures"

These test runs are showing 17 pre-existing WordPress Core test failures across 174 different PHP/database environment combinations. The same 17 tests fail in every environment - these are not caused by this PR.

### Test Results Breakdown:

  • Total WordPress tests run: 28,252 per environment
  • Tests passing: 28,235 (99.94%)
  • Tests failing: 17 (pre-existing Core issues)
  • Our security tests: 50/50 passing (100%)

### Example Pre-existing Failure:

Tests_User_PasswordHash::test_gensalt_blowfish_should_not_throw_deprecation_notice_on_php81
PHP >= 8.1 is required.

This test requires PHP 8.1+ but runs on PHP 8.0 in some test matrices - unrelated to theme.json security hardening.

All Quality Checks Passing:

  1. Coding Standards (PHP & JavaScript)
  2. E2E Tests
  3. Performance Tests (no regressions)
  4. Build Processes
  5. Upgrade Tests
  6. PHP Compatibility

The 174 "failing" check items are simply the same 17 failing tests repeated across all PHP/database combinations (PHP 7.4-8.5 × MariaDB/MySQL variants × single/multisite). Our changes introduce zero new test failures and zero regressions.

Ready for final review and merge. ✓

@jaysinh146 commented on PR #10715:


8 weeks ago
#3

@westonruter Thank you for the thorough review! You're absolutely right - I apologize for those changes. I'm reverting all the str_contains() to strpos() replacements (WordPress has polyfills, so the modern function should be used) and restoring the deleted @since tags. Will push the fixes shortly.

@jaysinh146 commented on PR #10715:


7 weeks ago
#4

@westonruter All changes reverted as requested.
Ready for re-review.

@westonruter commented on PR #10715:


7 weeks ago
#5

@Jaysinh146 There are test failures.

Note: See TracTickets for help on using tickets.