On Mon, Jun 12, 2017 at 10:48 PM, David Strauss <da...@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
