Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105275
MIME-Version: 1.0
References: <CAJRMtAG41c7tSDRO0H-pz+R=qxDva5h+7Q7mdkAUObN6fC+TPg@mail.gmail.com>
 <2e70065b-160b-7134-03ee-1a6025a6467c@gmail.com>
In-Reply-To: <2e70065b-160b-7134-03ee-1a6025a6467c@gmail.com>
Date: Mon, 15 Apr 2019 00:55:49 -0500
Message-ID: <CAJRMtAEZugqtpSRE9nKUd9=QtS8wt9RzxBsnj00aHW9DxXD2gQ@mail.gmail.com>
To: Stanislav Malyshev <smalyshev@gmail.com>
Cc: PHP Internals <internals@lists.php.net>, bishop@php.net
Content-Type: multipart/alternative; boundary="000000000000b0881305868b4efa"
Subject: Re: [PHP-DEV] PHP deserialization techniques offer rich pickings for
 security researchers
From: xwisdom@gmail.com (Raymond Irving)

--000000000000b0881305868b4efa
Content-Type: text/plain; charset="UTF-8"

Hi,

Thanks for responding to this issue.

Will calling getMetaData still parse and
execute malicious code?


;__
 Raymond



On Sun, 14 Apr 2019, 4:47 PM Stanislav Malyshev, <smalyshev@gmail.com>
wrote:

> Hi!
>
> > I came across this article which highlights a few issues with PHP
> > deserialization techniques:
> >
> >
> https://portswigger.net/daily-swig/phar-out-php-deserialization-techniques-offer-rich-pickings-for-security-researchers
>
> PHP serialization is not meant to be used with external or
> user-modifyable data. Looks like the crux of the issue is that phar uses
> unserialize() to read metadata, which is an insecure scenario since it
> is common to deal with external phar files and it's not obvious there
> can be serialized data. Particular Drupal exploit seems to be caused by
> insecure coding (one should not be able to give phar streams to system
> paths) but the general issue of reading phars being insecure stays.
>
> It is a bit problematic since there are no limitations in what can be
> stored in Phar metadata, so we can't really prohibit anything there
> without breaking BC. I would start with banning objects there (at least
> by default) but that again would be BC break if somebody did use objects
> there. More workable idea would be to not parse metadata unless
> getMetadata() is explicitly called. The chance of code that did not
> intend to use metadata to use this call is nil, thus eliminating the
> deserialization vector in most cases. OTOH, BC is kept since the
> metadata is still available for the code that needs it. I'll try to
> implement this soon.
> --
> Stas Malyshev
> smalyshev@gmail.com
>

--000000000000b0881305868b4efa--