Re: [PHP-DEV] [RFC] more secure unserialize()
And what about automatic un/serialize() of objects in $_SESSION?
People don't even see those function calls in their code, so dropping
the function/ality would be a wildly drastic move.
Nothing about it, the change is for unserialize() function.
OK. I thought of this as one core security issue with multiple
possible ways of getting a payload to the internal (C) unserialize
code. (If not, guess I could draw up an RFC for the other vector.)
It is harder to inject arbitrary objects into session storage than to
exploit blind-request-variable-unserialization type stuff (though the
latter can be a stepping stone to the former). But the potential
payoff in $_SESSION is so huge, I think having secure unserialize
for sessions is fully justified. Otherwise you're saying I can't
guarantee objects with killer wakeups/dtors were not injected via one
of the apps on my server, and I have no way to stop them from
instantiating magically provided they get through the right way.
We have to get away from mentality of if we need to modify some
behavior, we just put a variable in global state to control it.
Global state is the last resort, not the first one.
I guess it could be another argument to session_start() instead.
-- S.
P.S. Sure you'll shoot down this idea as well, but I think it would be
good if Filters had a corresponding validator, such as:
FILTER_VALIDATE_UNSERIALIZED: detect strings in PHP bytestream format.
Flags FILTER_ALLOW_SERIALIZED_SCALAR,
FILTER_ALLOW_SERIALIZED_NONOBJECT to fine-tune.
Otherwise, if you are still expecting bytestream format from the
client and want to detect tampering on input, you have to write a
best-guess regex to try to differentiate between 'Some:free { text;
}' and 'O:8:class:0:{}' and 'S:12...' etc.
.
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [RFC] more secure unserialize()
Stas, The fact is that people do use serialize() for data that may be accessible by the user (or made accessible by unrelated security issues, which may be upgraded in severity by this - e.g. from SQL injection to persistent code backdoor on the server). There are many ways to do things differently, and they are known. However, as I said, the fact is people do use serialize() and may not even realize the data aren't as secure as they are. That's why many security tools flag any object with dtor in application using unserialize as insecure. This is not a good situation, and presently there are no way to avoid it except dropping serialize() completely - which may not be an option is some cases and in any case would require serious changes to the production code. This enhancement is to fix this problem. It is not to change best practices or give advice on how to write the most secure system - it is to make existing code more secure easily. I definitely see your point, and don't disagree with it at all. Again, my concern is that people will then be tempted to use serialization to the client as it's safe (with these modifications). Which I think we should discourage for new code So what if we did this: We implement your RFC, but also put a warning in the docs that serialize() shouldn't be used in places where a user or third party can modify the output (to use json_encode() for those areas). That way we're not encouraging serialize to be used in places it shouldn't, but also give those with legacy codebases or really awkward use-cases the ability to be more secure... Thoughts?
[PHP-DEV] Add a constant to reflect --with-curlwrappers
Hey: there are some issues when people run some codes in a php which is compiled with --with-curlwrappers, like #61336, or the recently test script for #64433 (failed when curl wrappers enabled). I know, that the curl wrapper should act the same as php http wrapper, but for now, we need to provide the ability to user, that they can warn if they codes can not run with curl wrappers.. here are some really usages: https://github.com/UnionOfRAD/lithium/issues/59 and http://weizhifeng.net/wrong-with-curlwrappers.html I propose to add a constant : bool CURL_WRAPPERS_ENABLE or, any other better name... objections? thanks -- Laruence Xinchen Hui http://www.laruence.com/
Re: [PHP-DEV] [RFC] more secure unserialize()
On Sun, Mar 31, 2013 at 5:27 AM, Stas Malyshev smalys...@sugarcrm.comwrote: I think Stas proposes a solution to the problem and I think Anthony proposes a viable alternative. I would say that Anthony has found the shortest distance between the two points (the problem and the solution), however. The fact is that people do use serialize() for data that may be accessible by the user Yeah, well, the people who do that are also the ones that are unlikely to make use of the new parameters to secure themselves. In order to make them use of the new feature they have to be explicitly educated about it and in that case we can just as well educate them to use json_encode. In that regard, I don't think this proposal is particularly useful. JSON and serialize() are (inherently) different serialization formats with different use-cases. The former is rather restricted and as such safe to be provided by the user. The latter on the other hand aims at exactly replicating the structure. Using the latter format for the former task is in any case a bad idea. It's not like unserializing objects with dtors is the only issue that can turn up. serialize() also supports references and object-references, so one could probably use it quite easily to trigger some kind of infinite loop/recursion in the application. So, I personally don't see much value in this addition. Rather it could provide people an excuse to use the function on user-provided data, which is, as already mentioned, a bad idea. Even with this additional protection. Also I'd like to point out that unserialize() in the past had a relatively large number of different security vulnerabilities, so one should really, really not trust it with data of unknown origin. Internal classes can quite commonly be made to segfault with specially crafted serialization input. E.g. the user might think that, hey, DateTime is a safe class, let's allow unserializing that. Sounds legit doesn't it? Then let's remember those various serialization bugs that were recently fixed in DateTime (or the related classes, didn't really look at it). And then we would have a potentially exploitable segmentation fault :) Nikita
Re: [PHP-DEV] [RFC] more secure unserialize()
I think this RFC would worsen the problem of misusing the serialize round-trip. Even if we make the docs clearer, we'd still be sending the message that there's a new safer unserialize and some would certainly use that new feature to be more lax about guarding serialized structures. It also sets up a situation where altering either the whitelist or one of the classes in the whitelist could open a vulnerability that's not obvious. I'm also not convinced that this feature would spur developers to fix insecure code. But setting my arguments against, if the goal is to make unserialize() secure, then it should behave like a tripwire: fail loudly if a non-whitelisted class object is found. I think returning partially-usable values would gives devs more rope to hang themselves with. Re the 2nd arg, I'd make only two cases: * null is given: default behavior * non-null given: cast to array and that's the class whitelist. If the overall goal is to make the serialize/unserialize round-trip tamper-proof, we could build HMAC right into the API: add secret key args to both functions. No doubt the Suoshin patch already uses HMAC during encryption of the session data. Steve Clay -- http://www.mrclay.org/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] array_column bug and its fix
Hi, there seems to be a bug in array_column function. Look at this: http://3v4l.org/kZahr The variables $column_key and $index_key are strings after calling array_column which is probably wrong... :) I have fixed that - pull request: https://github.com/php/php-src/pull/316 Regards Jakub
[PHP-DEV] Re: [lists.php] Re: [PHP-DEV] [RFC] more secure unserialize()
JSON and serialize() are (inherently) different serialization formats with different use-cases [...] Yes, and json requires that all strings (including the keys) has to be valid utf-8, and I'm sure that's not always the case (serialize can use binary data in both places). -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: [lists.php] Re: [PHP-DEV] [RFC] more secure unserialize()
On 31/03/13 23:18, ALeX wrote: JSON and serialize() are (inherently) different serialization formats with different use-cases [...] Yes, and json requires that all strings (including the keys) has to be valid utf-8, and I'm sure that's not always the case (serialize can use binary data in both places). Yes, it is a problem. var_dump(json_encode(\xe1 - \xc3\xa1)); PHP Warning: json_encode(): Invalid UTF-8 sequence in argument in php shell code on line 1 string(4) null In a perfect world, all your input is utf-8, but sometimes what you get is in a different encoding... (and you still want to store it as-it-came in the first layer) -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Add a constant to reflect --with-curlwrappers
On Sun, Mar 31, 2013 at 6:25 AM, Laruence larue...@php.net wrote: Hey: there are some issues when people run some codes in a php which is compiled with --with-curlwrappers, like #61336, or the recently test script for #64433 (failed when curl wrappers enabled). I know, that the curl wrapper should act the same as php http wrapper, but for now, we need to provide the ability to user, that they can warn if they codes can not run with curl wrappers.. here are some really usages: https://github.com/UnionOfRAD/lithium/issues/59 and http://weizhifeng.net/wrong-with-curlwrappers.html I propose to add a constant : bool CURL_WRAPPERS_ENABLE The curl wrappers have always been a major pain, with plenty of bugs and we tend to forget to add context options there to match the standard wrapper on new feature. It has been marked as experimental since forever, and I think its time to face the failed experiment and remove it. -Hannes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
