Bug #75054 [Opn->Dup]: A Denial of Service Vulnerability was found when performing deserialization

Edit report at https://bugs.php.net/bug.php?id=75054&edit=1 ID: 75054 Updated by: nikic@php.net Reported by: varsleak at gmail dot com Summary: A Denial of Service Vulnerability was found when performing deserialization -Status: Open +Status: Duplicate Type: Bug Package: Variables related Operating System: Ubuntu 16.40 x64 PHP Version: 7.1.8 Block user comment: N Private report: N New Comment: Duplicate of bug #74103 and fixed by https://github.com/php/php-src/commit/1a23ebc1fff59bf480ca92963b36eba5c1b904c4. Previous Comments: ------------------------------------------------------------------------ [2017-08-12 11:18:11] nikic@php.net Automatic comment on behalf of nikita.ppv@gmail.com Revision: http://git.php.net/?p=php-src.git;a=commit;h=1a23ebc1fff59bf480ca92963b36eba5c1b904c4 Log: Fixed bug #74103 and bug #75054 ------------------------------------------------------------------------ [2017-08-10 10:03:50] nikic@php.net > "Treating unserialize issues as security creates the false sense that we > expect it to be secure, when we absolutely don't." - Zeev Suraski > > This interesting... Treating such issues as non-security would also create > the false sense that - we expect it to be secure now because we just updated > a warning box in the documentation - nevertheless unsafe legacy code stays > the same. The warning box has been present for many years already. We've been telling users that unserialize() is fundmantally insecure for a long time. However, the messaging was inconsistent, with documentation saying that you cannot use it, but the security policy still treating it like you could -- this leaves the impression that it's okay to do this as long as you keep your PHP installation updated. Which is very far from the truth, as you are probably aware. > CVE assignment is a mechanism to keep track of issues and keep users updated, > so they are aware of potential issues. They are not there just to make > developers feel bad about the code or whatsoever. I don't think that CVEs about specific issues in unserialize() are useful at this point. The only thing users need to know is that if they are using unserialize() on untrusted data, they are vulnerable. Even assuming all the issues we are currently aware of are fixed, I could still say with confidence that there are more vulnerabilities in there and it would not even be particularly hard to find them. In combination with user code implementing Serializable, there are issues that we categorically cannot fix without breaking compatibility. > As was done in the Facebook HHVM fork, they have changed the more risky > wddx_deserialize() code from native C/++ to PHP/HH, this IMHO is more > helpful than dismissing a whole class of memory bugs as non-issues. I'm not familiar with wddx, so can't comment on that. I do know though that HHVM has made some breaking changes to unserialize() to mitigate issues with Serializable. Maybe we should consider removing support for shared serialization contexts as well. But in any case, that's not something that would be applicable to current stable versions of PHP. ------------------------------------------------------------------------ [2017-08-10 09:33:35] l dot wei at ntu dot edu dot sg Thanks for the link and clarification. "Treating unserialize issues as security creates the false sense that we expect it to be secure, when we absolutely don't." - Zeev Suraski This interesting... Treating such issues as non-security would also create the false sense that - we expect it to be secure now because we just updated a warning box in the documentation - nevertheless unsafe legacy code stays the same. CVE assignment is a mechanism to keep track of issues and keep users updated, so they are aware of potential issues. They are not there just to make developers feel bad about the code or whatsoever. As was done in the Facebook HHVM fork, they have changed the more risky wddx_deserialize() code from native C/++ to PHP/HH, this IMHO is more helpful than dismissing a whole class of memory bugs as non-issues. ------------------------------------------------------------------------ [2017-08-10 09:00:05] nikic@php.net For context, please see this recent discussion on the PHP internals list: https://externals.io/message/100147 The situation is basically that given the current serialization format, it is unlikely that unserialize() will EVER be suitable for use on untrusted input. There are some very fundamental issues which are getting papered over as new bug reports come in, but the real issue is in the format itself -- without changing the format, it appears to be impossible to make unserialize() fully secure. This is why there is a big red box in the unserialize() documentation: http://php.net/unserialize ------------------------------------------------------------------------ [2017-08-10 07:42:56] l dot wei at ntu dot edu dot sg Simply dismiss these issues by suggesting a "best practice" does not seem to improve the situation in systems built on the unsafe deserialization APIs. If deprecating them is too costly for compatibility, maybe fixing such issues as soon as they get reported is the best way to go ? Just two cents. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=75054 -- Edit this bug report at https://bugs.php.net/bug.php?id=75054&edit=1