Double escaping wp_user-settings

[phatkoala]

Fresh install of WordPress with no plugins and using default Twenty Twenty Two theme.

Either directly insert test data;

INSERT INTO wp_usermeta (user_id, meta_key, meta_value) VALUES (1, 'wp_user-settings', 'foo=1&bar=1');

Or activate this plugin;

<?php
/*
Plugin Name: Ampersand Test
*/
register_activation_hook( __FILE__, function(){
    set_user_setting( 'foo', 1 );
    set_user_setting( 'bar', 1 );
} );

The name of the attributes and their values are completely arbitrary and have no impact on the behaviour.

Now go to your "Profile" page (/wp-admin/profile.php) and press "Update Profile". Wait 5 seconds and repeat.

The value of wp-user_settings in the database and COOKIE is being double escaped (escaped on read and escaped on write).

Therefore the value of wp-user_settings does this;

• foo=1&bar=1
• foo=1&ampbar=1
• foo=1&ampampbar=1
• foo=1&ampampampbar=1
• and so forth

Once the value of wp-user_settings becomes too long and/or combined with a user using the same browser with multiple logins, the length of the COOKIE(s) will become too large and the request header will be rejected by Apache/Nginx.

[azouamauriac]

Hi @phatkoala welcome to trac! Thanks for the report and the steps to reproduce it. I'm able to reproduce it.

Hope it will be fixed ASAP.

Ampersands (&) are being double escaped in wp_user-settings. This bug fix adds a regular expression to remove escaped ampersands from wp_user-settings.

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

[costdev]

Test Report

Env

• Server: Apache (Linux)
• WordPress: 6.0-alpha-52448-src
• Browser: Chrome 99.0.4844.51
• OS: Windows 10
• Theme: Twenty Twenty-One
• Plugins: None activated.

Steps to reproduce

1. Save the plugin code in this ticket's description to wp-content/plugins/ampersand_test/ampersand_test.php.
2. Navigate to Plugins > Installed Plugins.
3. Activate the Ampersand Test plugin.
4. Navigate to Users > Profile.
5. Open the database and navigate to {prefix}_usermeta.
6. Filter for wp_user-settings.
7. See that the value contains &ampfoo=1&ampbar=1.
8. Refresh the Profile page.
9. Repeat steps 5 and 6.
10. See that the value contains &amp&ampfoo=1&amp&ampbar=1.

Cleanup

1. Edit the database entry and remove &amp&ampfoo=1&amp&ampbar=1.
2. Save.
3. Navigate to Plugins > Installed Plugins.
4. Deactivate the Ampersand Test plugin, then reactivate it.

Steps to test ​PR 2458

1. Checkout ​PR 2458.
2. Open the database and navigate to the {prefix}_usermeta table.
3. Filter for wp_user-settings.
4. See that the value contains &foo=1&bar=1.
5. Refresh the Profile page.
6. See that the value still contains &foo=1&bar=1.

Results

1. Issue reproduced. ✅
2. ​PR 2458 resolves the issue. ✅

Notes

1. Introduced in [8784].
2. Milestoning for 6.0 to get this some visibility.
3. Adding dev-feedback to verify that this approach has no unintended side effects / BC breaks.
4. Adding has-testing-info and needs-testing to get some tester creativity on this.

[azouamauriac]

Hi thanks for test report, and thanks @phatkoala for the PR

Replying to costdev:

2. ​PR 2458 resolves the issue. ✅

He updated the PR, and when I tested it, here is the result: ​before the last PR: ​https://prnt.sc/Bej0-MVvbgSj ; ​after the last PR : ​https://prnt.sc/ZZ53foh-Xs3b; So I think you should use commit link instead of PR since it can be updated anytime and commiters can be involved in mistakes. I think it is ​this commit you have tested and agreed(I guess). I tested it too, and it fixes the bug, but I am not so fan of the way, while I am still thinking about how to fix it properly, I agree with the commit though.

1. Introduced in [8784].

IMO the bug is due to ​this line in this function
​WP_User::__get
(if you comment ​this line everything will work fine) introduced in [18597].
Also, just for information, I've made some tests on 4.9.20 version and the bug is not present there, while it's present in 5.8 where I've done some tests too.

[costdev]

With 6.0 Beta 1 tomorrow and as this ticket still needs more investigation, I'm moving this to the Future Release milestone.