On Mon, Jun 12, 2017 at 10:48 PM, David Strauss <david@davidstrauss.net>
wrote:
> Despite providing class whitelisting [1] and documentation about warnings
> about security impacts [2], we continue to see vulnerable uses of
> unserialize() in Drupal modules [3] and partially effective attempts to
> mitigate vulnerabilities from user-supplied, serialized data [4].
>
> Whitelisting legal classes for unserializing data is, unfortunately, not
> seeing widespread uptake in community-created code that I review. It also
> doesn't push us toward more secure defaults shipping with OS packages and
> on PHP-supporting platforms. An additional option that would generally work
> with existing, legitimate use but would also block use for exploits could
> turn that tide.
>
> I propose adding a new key to the $options parameter for unserialize(). The
> new key would be "exception_on_side_effects", have a Boolean value, and
> would (if true) cause unserialization to halt (and an exception to be
> thrown) if any of the data being unserialized contains objects with magic
> methods that will automatically execute on object wakeup, destruction, or
> other events that the PHP interpreter (almost) always triggers.
>
> To help push toward better defaults, I also suggest adding a related
> configuration option:
>
> Name: unserialize_side_effect_protection
> Default: 0
> Changeable: PHP_INI_ALL
>
> If enabled, it would cause "exception_on_side_effects" to be enabled by
> default on all unserialize() calls that don't specify "allowed_classes" or
> override the default. By making it PHP_INI_ALL, frameworks could lock down
> usage during bootstrap (or at least before reading request data).
>
> I am a member of the Drupal Security Team and would also be the implementer
> of this feature. My username on the wiki is "dts", and I'm requesting RFC
> karma.
>
> [1] https://wiki.php.net/rfc/secure_unserialize
> [2] https://secure.php.net/manual/en/function.unserialize.php
> [3] https://www.ambionics.io/blog/drupal-services-module-rce
> [4]
> https://github.com/WordPress/WordPress/blob/efab6e06cae3ed14c6b681570dfd5f
> 81917d9f9c/wp-includes/functions.php#L341-L394
Hi,
how do you intend to handle internal classes in this regard? Internal
classes (if they support serialization at all) will pretty much always use
either __wakeup or Serializable::unserialize(). Does that mean that if
exception_on_side_effects is enabled, most internal classes will be
prohibited?
Regards,
Nikita
