Make WordPress Core

Opened 3 months ago

Last modified 7 weeks ago

#59233 new task (blessed)

Improve error handling for unserialize()

Reported by: jrf's profile jrf Owned by:
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: General Keywords: php83 2nd-opinion
Focuses: php-compatibility Cc:

Description

From https://core.trac.wordpress.org/ticket/59231:

Make unserialize() emit a warning for trailing bytes

While based on the current test suite, WP is not directly affected by this, the `maybe_unserialize()` function could still be confronted by data with trailing bytes.

However, the call to the PHP native unserialize() within maybe_unserialize() silences all (PHP 8.0+: non-fatal) errors, so this new warning will not affect WP or its ecosystem as long as the maybe_unserialize() function is used.

Having said that, a critical look at maybe_unserialize() may be warranted as the new warning in PHP is related to security issues discovered in other projects, so WP may want to consider rejecting unserialization for data throwing this warning.

Also note that there are 7 uses of unserialize() in total within WP Core, one within maybe_unserialize(), but the function is also used in 6 other places and 5 of those do not use error silencing.

Improve unserialize() error handling

This, again, affects the `maybe_unserialize()` function and this time, the code should probably be adjusted to handle the new errors which unserialize() can now throw.

The change does not affect unserializing valid data, but in the case of invalid data, the type of and severity of the notices/warnings/catchable exceptions have been changed.

All 7 uses of unserialize() in WP Core should be reviewed and for the 6 uses outside of the maybe_unserialize() function, it should be reviewed whether they can/should switch to using maybe_unserialize() and/or whether they should get their own (improved) error handling.

Change History (2)

#1 @jrf
3 months ago

  • Keywords 2nd-opinion added

#2 @hellofromTonya
7 weeks ago

  • Milestone changed from 6.4 to 6.5

6.4 RC1 is happening a few hours. Moving this to 6.5.

Note: See TracTickets for help on using tickets.