Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:99499
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.181 as permitted sender)
MIME-Version: 1.0
Date: Mon, 12 Jun 2017 20:48:23 +0000
Message-ID: <CABpmRD4J1CRz0uBrew9kOrfn2fysaCoE8greViz2FTQqtubEhg@mail.gmail.com>
To: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a114876d2665bc10551c97192"
Subject: Intended RFC Proposal: Side-Effect Filtering for unserialize()
From: david@davidstrauss.net (David Strauss)

--001a114876d2665bc10551c97192
Content-Type: text/plain; charset="UTF-8"

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/efab6e06cae3ed14c6b681570dfd5f81917d9f9c/wp-includes/functions.php#L341-L394

--001a114876d2665bc10551c97192--