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: |
|
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
@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:
- Coding Standards (PHP & JavaScript)
- E2E Tests
- Performance Tests (no regressions)
- Build Processes
- Upgrade Tests
- 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.
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:
New sanitization methods in WP_Theme_JSON:
Test coverage:
Props: villu164
Fixes #62224
Trac ticket: https://core.trac.wordpress.org/ticket/62224