Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:100253
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain kelunik.com from 81.169.146.217 cause and error)
MIME-Version: 1.0
In-Reply-To: <CAF+90c_bxYCEMWSL7QO2BSTq6gOVwkmTnCj1OEt3xo-7QHTuAg@mail.gmail.com>
References: <72449705-4871-d317-857e-388f53800bbd@gmx.de> <CAF+90c_bxYCEMWSL7QO2BSTq6gOVwkmTnCj1OEt3xo-7QHTuAg@mail.gmail.com>
Date: Fri, 18 Aug 2017 12:12:46 +0200
Message-ID: <CANUQDCgHxQbihg8Q3Qe+inY9uJDr4xB9x=xU9_6WyMUe_ft+Ag@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: "Christoph M. Becker" <cmbecker69@gmx.de>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="001a114bb6f4e554100557045ee5"
Subject: Re: [PHP-DEV] [RFC] Deprecate class instance deserialization in WDDX
From: me@kelunik.com (Niklas Keller)

--001a114bb6f4e554100557045ee5
Content-Type: text/plain; charset="UTF-8"

>
> On Tue, Aug 15, 2017 at 6:54 PM, Christoph M. Becker <cmbecker69@gmx.de>
> wrote:
>
> > Hi internals!
> >
> > Due to the recent discussion regarding WDDX serialization and security
> > (<http://marc.info/?l=php-internals&m=150245739612076&w=2>), I've
> > written an RFC that proposes to deprecate class instance deserialization
> > in WDDX:
> >
> > <https://wiki.php.net/rfc/wddx-deprecate-class-instance-deserialization>
> >
> > I hereby put this RFC under discussion.
> >
> > Note that I have fully intentional left out issues like moving the WDDX
> > extension to PECL, actually removing the class instance deserialization
> > and the `wddx` session serialization handler, to eschew lengthy
> > discussions, because I would like to see the deprecation already
> > happening in *PHP 7.2*, since this is a rather sensitive issue.
> >
>
> As I've already said in the previous thread, I don't think this is the
> right way to go about this issue. Instead we should push harder to remove
> this extension entirely.
>
> Let me recapitulate what the issues with this extension are:
>
> 1. Security (object injection): __wakeup() can be triggered by untrusted
> input, usually exploitable with enough effort.
> 2. Security (other): While WDDX doesn't have any of the fundamental issues
> of unserialize(), the extension has a very bad track record where security
> is concerned. For two recent relevant bugs see #74145 (segfault on 5.6) and
> #73173 (memleak). These are by no means isolated occurrences, the wddx
> extension has seen quite a few security patches in the past. Maybe
> everything is fixed now? I wouldn't bet on it.
> 3. Irrelevance: It's 2017, nobody uses WDDX. (With the usual qualifications
> on "nobody".)
>
> On top of that the API is quite ridiculous, with wddx_add_vars() and
> wddx_serialize_vars() taking variable names (!!!) to serialize. This API
> must be from a time when register_globals not only still existed but was
> probably the preferred way of doing things.
>
> What this RFC solves is the first point, in a backwards-compatibility
> breaking way. Even with this resolved, I would still be wary of using this
> on untrusted input due to the second point. The third point just means that
> we shouldn't waste time on elaborate solutions.
>
> Which is why I would suggest:
>  1. Deprecate the entire extension in PHP 7.2.
>  2. Unbundle it in PHP 7.3.
>  3. (Optional -- someone who needs it can do it) Provide a PHP polyfill
> implementation for wddx_serialize_value and wddx_deserialize.
>

Why not immediately unbundle it in PHP 7.2?

Regards, Niklas

--001a114bb6f4e554100557045ee5--