Skip to content

Route the last two raw config writes through wp_cache_setting() #1095

Description

@donnchawp

Summary

wp_cache_setting() now builds its config line with var_export() (#1092), so every caller of it writes a value the config file can read back. Two writes still bypass it and interpolate raw into PHP source. Both should route through wp_cache_setting() rather than gain another copy of the metacharacter strip.

There are ~70 direct wp_cache_replace_line() calls outside wp-cache-phase2.php. All but these two carry numeric, (int)-cast, whitelisted or format-validated values, so this is a two-site job, not a sweep.

1. $wp_cache_mobile_groupsinc/htaccess.php:48

wp_cache_replace_line( '^ *\$wp_cache_mobile_groups', "\$wp_cache_mobile_groups = '" . implode( ', ', $mobile_groups ) . "';", $wp_cache_config_file );

Fed from apply_filters( 'cached_mobile_groups', array() ) (inc/admin-ui.php:629), so the value is whatever a third-party filter returns.

This one is more than an escaping gap — the writer and the reader disagree about the type. wp_cache_mobile_group() (wp-cache-phase2.php) expects a nested array:

foreach ( (array) $wp_cache_mobile_groups as $name => $group ) {
    foreach ( (array) $group as $browser ) {

and the documented shape at inc/admin-ui.php:630 agrees:

// mobile_groups = array( 'apple' => array( 'ipod', 'iphone' ), 'nokia' => array( 'nokia5800', 'symbianos' ) );

But the writer implode()s it into a string. For the documented nested shape that yields the literal 'Array' plus an "Array to string conversion" notice — so the grouping feature does not work as documented today, independently of any escaping concern.

The fix is the same as the escaping fix: pass the array to wp_cache_setting(), which takes the array branch and stores a real array literal via var_export(). Note the two lines above it in the same function already do this:

wp_cache_setting( 'wp_cache_mobile_browsers', $mobile_browsers );
wp_cache_setting( 'wp_cache_mobile_prefixes', $mobile_prefixes );

Worth checking whether anything else reads $wp_cache_mobile_groups as a string before changing the stored type, and whether an existing config holding the string 'Array' needs handling on upgrade.

2. $wptouch_exclude_uaplugins/wptouch.php:97

$browsers = implode( ',', bnc_wptouch_get_exclude_user_agents() );
wp_cache_replace_line( '^ *\$wptouch_exclude_ua', "\$wptouch_exclude_ua = '$browsers';", $wp_cache_config_file );

An implode of a third-party plugin's user-agent list, interpolated raw. Reachable only with WPtouch installed and an apostrophe or backslash somewhere in its exclude list — low, but it is a straight wp_cache_setting( 'wptouch_exclude_ua', $browsers ) swap.

plugins/wptouch.php:15 writes $wptouch_browsers the same way; that value comes from bnc_wptouch_get_user_agents() and deserves the same treatment while the file is open.

Why route rather than strip

The three existing metacharacter strips (inc/admin-ui.php:433, inc/settings-forms.php:358,368) are what this codebase reached for last time. That approach is why the REST cache_path handler and six other string settings went unprotected for years — a strip has to be remembered at every new call site, and it was not. Routing through wp_cache_setting() makes the guarantee structural, and deletes a copy of the strip pattern rather than adding a fourth.

Not in scope here

With the sink fixed, the strips at inc/settings-forms.php:358 and :368 are no longer load-bearing for $wp_cache_debug_ip and $wp_super_cache_front_page_text, and they silently mangle legitimate input — an apostrophe or parentheses in the front-page notification text still disappear. Removing them is a user-visible behaviour change with its own risk and belongs in its own issue.

Suggested labels: bug, ready-for-agent.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugready-for-agentFully specified, ready for an AFK agent

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions