Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:99523
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.223.170 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CABpmRD4J1CRz0uBrew9kOrfn2fysaCoE8greViz2FTQqtubEhg@mail.gmail.com>
References: <CABpmRD4J1CRz0uBrew9kOrfn2fysaCoE8greViz2FTQqtubEhg@mail.gmail.com>
Date: Thu, 15 Jun 2017 11:35:12 +0200
Message-ID: <CAF+90c-7PNE3i5DwcCyQ2oUwQGKmmbYgqt6Vu5w_th71LjMY4A@mail.gmail.com>
To: David Strauss <david@davidstrauss.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a113ee16ab705990551fc623c"
Subject: Re: [PHP-DEV] Intended RFC Proposal: Side-Effect Filtering for unserialize()
From: nikita.ppv@gmail.com (Nikita Popov)

--001a113ee16ab705990551fc623c
Content-Type: text/plain; charset="UTF-8"

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

--001a113ee16ab705990551fc623c--