Adding Defense-in-Depth against PHP Object Injection

[FrancescoCarlucci]

The WordPress core function maybe_unserialize() uses PHP unserialization in an insecure way, and I think we are all well aware of that.

The reason is that PHP unserialize automatically instantiates any object received as input, and if the object uses magic methods it can lead to RCE, file deletion and other dangerous security issues. In the last year there were hundreds of security vulnerabilities caused by unsafe use of maybe_unserialize(), and with this ticket I want to propose a fix at core level, which in my opinion is easy straightforward :)

PHP unserialize() accepts a second parameter allowed_classes that can be set to false to disallow automatic class instantiation, or can be restricted to specific classes.

So we can tweak maybe_unserialize() this way:

<?php
function maybe_unserialize( $data ) {
if ( is_serialized( $data ) ) { // Don't attempt to unserialize data that wasn't serialized going in.
  $allowed_classes = apply_filters( 'maybe_unserialize_allowed_classes', false );
  return @unserialize( trim( $data ), [ 'allowed_classes' => $allowed_classes ] );
}
    
return $data;
}

This way allowed_classes is false by default and can be hooked to true or a set of classes, this would be a security-first approach.

Other ways, we can have a backward-compatibility first approach:

<?php
function maybe_unserialize( $data ) {
if ( is_serialized( $data ) ) { // Don't attempt to unserialize data that wasn't serialized going in.
  $allowed_classes = apply_filters( 'maybe_unserialize_allowed_classes', true );
  return @unserialize( trim( $data ), [ 'allowed_classes' => $allowed_classes ] );
}
    
return $data;
}

And then site owners can harden their WP instance with this one-liner:

<?PHP

add_filter( 'maybe_unserialize_allowed_classes', '__return_false' );

To restrict instantiations to specific classes, plugins could hook in like this:

<?php
add_filter( 'maybe_unserialize_allowed_classes', function() {
    return ['MyClass', 'AnotherClass', 'SomeOtherClass'];
});

Anything else needed to merge it in the core? Does anyone see anything that could go wrong?

Thanks,
Francesco

This pull request introduces the maybe_unserialize_allowed_classes filter to enhance security by providing control over which classes can be instantiated during unserialization. This helps to mitigate potential PHP Object Injection vulnerabilities.

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

[siliconforks]

This is similar to #37757.

[FrancescoCarlucci]

Replying to siliconforks:

This is similar to #37757.

Thanks for letting me know! Should I keep the discussion on the other ticket?

[johnbillion]

Agreed, let's close this off as a duplicate of #37757. Thanks!