Everything

php

[RFC] Serialize filtering

php.internals

Stas Malyshev

11 years ago

Hi!

I'd like to have a vote on unserialize() improvement proposal outlined here:
https://wiki.php.net/rfc/secure_unserialize

soon-ish, but since discussion on it has been more than a year ago I'd
like to give it some prior notice and some time to re-consider. I still
think it is a good improvement, not fixing all problems but allowing to
fix some at reasonable cost. I've added some outline of arguments
discussed before, but still open for comments.  The patch is probably
outdated but I'll fix it if it's accepted, if not I don't want to spend
time on it. I'd like to have a vote sometime next week, but if there's
more discussion it can be postponed.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

Dmitry Stogov

11 years ago

Hi Stas,

I'm not sure if this new argument to unserialize() is intuitive.
May be better to use separate functions - unserialize_filtered() or
something similar.

Thanks. Dmitry.

On Mon, Oct 27, 2014 at 11:03 AM, Stas Malyshev <smalyshev@sugarcrm.com>
wrote:

Andrew Faulds

11 years ago

> On 27 Oct 2014, at 08:03, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
>
> I'd like to have a vote on unserialize() improvement proposal outlined here:
> https://wiki.php.net/rfc/secure_unserialize

Suggestion on improving the API: Why bother with three values?

If there’s no parameter, use current behaviour. If there’s an array, it’s allowed classes. If that array is empty, obviously there are no allowed classes.

--
Andrea Faulds
http://ajf.me/

Andrew Faulds

11 years ago

> On 27 Oct 2014, at 20:38, Andrea Faulds <ajf@ajf.me> wrote:
>
>
>> On 27 Oct 2014, at 08:03, Stas Malyshev <smalyshev@sugarcrm.com> wrote:
>>
>> I'd like to have a vote on unserialize() improvement proposal outlined here:
>> https://wiki.php.net/rfc/secure_unserialize
>
> Suggestion on improving the API: Why bother with three values?
>
> If there’s no parameter, use current behaviour. If there’s an array, it’s allowed classes. If that array is empty, obviously there are no allowed classes.

For example:

// this will unserialize everything as before
$data = unserialize($foo);
// this will convert all objects into __PHP_Incomplete_Class object
$data = unserialize($foo, []);
// this will convert all objects except ones of MyClass and MyClass2 into __PHP_Incomplete_Class object
$data = unserialize($foo, ["MyClass", “MyClass2"]);

--
Andrea Faulds
http://ajf.me/

Stas Malyshev

11 years ago

Hi!

> Suggestion on improving the API: Why bother with three values?
>
> If there’s no parameter, use current behaviour. If there’s an array,
> it’s allowed classes. If that array is empty, obviously there are no
> allowed classes.

You are right, empty array probably would do the same thing.

--
Stanislav Malyshev, Software Architect
SugarCRM: http://www.sugarcrm.com/

Remi Collet

11 years ago

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Le 27/10/2014 09:03, Stas Malyshev a écrit :

> Hi!
>
> I'd like to have a vote on unserialize() improvement proposal
> outlined here: https://wiki.php.net/rfc/secure_unserialize
>
> soon-ish, but since discussion on it has been more than a year ago
> I'd like to give it some prior notice and some time to re-consider.
> I still think it is a good improvement, not fixing all problems but
> allowing to fix some at reasonable cost. I've added some outline of
> arguments discussed before, but still open for comments.  The patch
> is probably outdated but I'll fix it if it's accepted, if not I
> don't want to spend time on it. I'd like to have a vote sometime
> next week, but if there's more discussion it can be postponed.
>

+1 as this seems to have a real benefit for security

(implementation detail such as function or option name are... detail)

Remi.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlRPmL8ACgkQYUppBSnxahhLrQCePtlnYkVuhSNFPF+pvjZ+DNZX
GaoAoLXKHYtbblmT9G0Y/jPRDgUtgABT
=mE9N
-----END PGP SIGNATURE-----