Opened 3 years ago
Last modified 3 years ago
#55456 new defect (bug)
Double escaping wp_user-settings
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 2.7 |
Component: | Users | Keywords: | has-patch dev-feedback has-testing-info needs-testing |
Focuses: | Cc: |
Description
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&bar=1
- foo=1&ampbar=1
- foo=1&ampampbar=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.
Change History (6)
This ticket was mentioned in PR #2458 on WordPress/wordpress-develop by PhatKoala.
3 years ago
#2
- Keywords has-patch added; needs-patch removed
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
#3
follow-up:
↓ 4
@
3 years ago
- Keywords dev-feedback has-testing-info needs-testing added
- Milestone changed from Awaiting Review to 6.0
- Version changed from 5.9.2 to 2.7
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
- Save the plugin code in this ticket's description to
wp-content/plugins/ampersand_test/ampersand_test.php
. - Navigate to
Plugins > Installed Plugins
. - Activate the
Ampersand Test
plugin. - Navigate to
Users > Profile
. - Open the database and navigate to
{prefix}_usermeta
. - Filter for
wp_user-settings
. - See that the value contains
&foo=1&bar=1
. - Refresh the
Profile
page. - Repeat steps 5 and 6.
- See that the value contains
&&foo=1&&bar=1
.
Cleanup
- Edit the database entry and remove
&&foo=1&&bar=1
. - Save.
- Navigate to
Plugins > Installed Plugins
. - Deactivate the
Ampersand Test
plugin, then reactivate it.
Steps to test PR 2458
- Checkout PR 2458.
- Open the database and navigate to the
{prefix}_usermeta
table. - Filter for
wp_user-settings
. - See that the value contains
&foo=1&bar=1
. - Refresh the
Profile
page. - See that the value still contains
&foo=1&bar=1
.
Results
- Issue reproduced. ✅
- PR 2458 resolves the issue. ✅
Notes
- Introduced in [8784].
- Milestoning for 6.0 to get this some visibility.
- Adding
dev-feedback
to verify that this approach has no unintended side effects / BC breaks. - Adding
has-testing-info
andneeds-testing
to get some tester creativity on this.
#4
in reply to:
↑ 3
@
3 years ago
Hi thanks for test report, and thanks @phatkoala for the PR
Replying to costdev:
- 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.
- 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.
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.