[wp-trac] [WordPress Trac] #37757: Add `allowed_classes` to `maybe_unserialize` When WordPress is running on PHP 7+

[WordPress Trac]

#37757: Add `allowed_classes` to `maybe_unserialize` When WordPress is running on
PHP 7+
-------------------------------------------------+-------------------------
 Reporter:  chrisguitarguy                       |       Owner:  (none)
     Type:  enhancement                          |      Status:  new
 Priority:  normal                               |   Milestone:  Awaiting
                                                 |  Review
Component:  Security                             |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  has-patch has-unit-tests 2nd-        |     Focuses:
  opinion                                        |
-------------------------------------------------+-------------------------
Changes (by johnbillion):

 * keywords:  has-patch has-unit-tests => has-patch has-unit-tests 2nd-
               opinion


Comment:

 I think the proposed filter will break a lot of things.

 If a plugin serializes `class Foo {}`, unserialization will break if
 another plugin hooks into the `maybe_unserialize_allowed_classes` filter
 and adds `Bar` to the resulting array of classes names, because `Foo`
 won't be present. This will also break unserialization of all WordPress
 core classes and `stdClass` instances that are serialized but aren't
 present in the `maybe_unserialize_allowed_classes` filter return value.

 Example code that breaks everything:

 {{{#!php
 <?php
 add_filter( 'maybe_unserialize_allowed_classes', function( $allowed ) {
   if ( ! is_array( $allowed ) ) {
     $allowed = [];
   }

   $allowed[] = 'Bar';

   return $allowed;
 } );
 }}}

 This example code also demonstrates that plugins will need to include
 logic to handle a boolean value. A plugin would be unable to determine if
 a value of `true` is the default value or one explicitly set by another
 plugin.

 The original patch on this ticket proposes adding an `$options` argument
 to `maybe_unserialize()` instead. This is useful when code calls
 `maybe_unserialize()` directly, but not useful when code calls functions
 that make use of it such as `get_option()`, `get_transient()`, the various
 `get_*_meta()` functions, etc.

 Perhaps a better approach would be to add an `$options` argument to all
 functions that can call `maybe_unserialize()` or `unserialize()` and pass
 the `$options` argument through.

 Before doing that though, I think we need to assess the exact threat and
 whether introducing that argument would address it.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/37757#comment:12>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform