php-memcached-dev / php-memcached

memcached extension based on libmemcached library
Other
988 stars 323 forks source link

Restricting unserialize() for security #395

Open tstarling opened 6 years ago

tstarling commented 6 years ago

We've been considering (at https://phabricator.wikimedia.org/T169328 etc.) how to protect MediaWiki from potential escalation from memcached access to arbitrary execution. The issue is that if an attacker can access memcached, they could write a value which, when unserialized, causes execution of arbitrary code in __wakeup() or another magic method. This depends on there being a vulnerable class, but the attack surface is very large.

OPT_SERIALIZER is not sufficient at present since the PHP client will always call php_var_unserialize() if the memcached server specifies the relevant flag.

Migrating MediaWiki to JSON serialization would be a lengthy undertaking, and would warrant some complexity in migration support. One possibility would be to have an option which specifies a userspace unserializer, e.g.

$memcached->setOption( Memcached::OPT_UNSERIALIZE_FUNCTION, 'my_unserialize' ) );

The userspace function would be called with val_type and payload, and would be expected to return the unserialized value, replacing the default handling in s_unserialize_value(), for example:

function my_unserialize( $type, $payload ) {
  switch ( $type ) {
    case Memcached::SERIALIZER_PHP:
      trigger_error( 'Using deprecated PHP serializer', E_USER_WARNING );
      return unserialize( $payload, [ 'allowed_classes' => false ] );
    case Memcached::SERIALIZER_JSON:
      return json_decode( $payload );
    default:
      trigger_error( 'Invalid memcached serialization format', E_USER_WARNING );
      return false;
  }
}

In the long term and for non-MediaWiki users, a more convenient alternative could be e.g. OPT_SECURE_UNSERIALIZE, a boolean option which when true would disable all branches of s_unserialize_value() which may lead to arbitrary code execution.

sodabrew commented 6 years ago

The user-defined callback function is a neat idea!