php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #81153 unserialize error calls __destruct before __wakeup
Submitted: 2021-06-17 08:49 UTC Modified: 2021-06-17 11:36 UTC
Votes:3
Avg. Score:3.0 ± 1.6
Reproduced:1 of 1 (100.0%)
Same Version:1 (100.0%)
Same OS:1 (100.0%)
From: j7ur8 at qq dot com Assigned:
Status: Open Package: Class/Object related
PHP Version: 7.3.28 OS: ALL
Private report: No CVE-ID: None
View Developer Edit
 [2021-06-17 08:49 UTC] j7ur8 at qq dot com 
Description:
------------
bad unserialize string makes __wakeup ineffective.

Success:
    7.0.15 - 7.0.33, 7.1.1 - 7.1.33, 7.2.0 - 7.2.34, 7.3.0 - 7.3.28, 7.4.0 - 7.4.16, 8.0.0 - 8.0.3

Fail:
    5.0.0 - 5.0.5, 5.1.0 - 5.1.6, 5.2.0 - 5.2.17, 5.3.0 - 5.3.29, 5.4.0 - 5.4.45, 5.5.0 - 5.5.38, 5.6.0 - 5.6.40, 7.0.0 - 7.0.14, 7.1.0



Test script:
---------------
// https://3v4l.org/4nZUm
<?php
class D{

	public $flag=True;
	public function __get($a){
		if($this->flag){
			echo 'flag';
		}else{
			echo 'hint';
		}
	}

	public function __wakeup(){
		$this->flag = False;
	}
}

class C{

		public function __destruct(){
		echo $this->c->b;
	}
}

@unserialize('O:1:"C":1:{s:1:"c";O:1:"D":0:{};N;}');

Expected result:
----------------
hint

Actual result:
--------------
flag

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-06-17 08:59 UTC] requinix@php.net
-Status: Open +Status: Duplicate
 [2021-06-17 08:59 UTC] requinix@php.net 
You already reported this.
 [2021-06-17 09:00 UTC] j7ur8 at qq dot com 
this is another one!
 [2021-06-17 09:18 UTC] requinix@php.net
-Summary: bypass __wakeup +Summary: unserialize error calls __destruct before __wakeup -Status: Duplicate +Status: Open
 [2021-06-17 09:18 UTC] requinix@php.net 
Yes, this is different. Please try to give a correct bug summary next time.

https://3v4l.org/0YvfT

> Notice: unserialize(): Error at offset 31 of 33 bytes in /in/0YvfT on line 22
> C::__destruct
> D::__get(b)
> D::__wakeup
> D::__destruct

Feels like the order of operations should have been

> D::__wakeup - because the object was successfully created
> C::__destruct - unserialization failed, begin destruction
> C::__get(b) - because of the code
> D::__destruct - cleaning up after C was destroyed
 [2021-06-17 11:01 UTC] nikic@php.net
-Status: Open +Status: Feedback
 [2021-06-17 11:01 UTC] nikic@php.net 
This needs some justification as to why this is supposed to be a bug. I don't think unserialize() guarantees any particular order here, and the order in which things are called looks reasonable to me (other orders would be reasonable as well).
 [2021-06-17 11:33 UTC] cmb@php.net 
@nikic, see <https://3v4l.org/UOHAh>.  It is surprising that
__get() is called before __wakeup(), especially since __wakeup()
would initialize $b, so __get() is not supposed to be called at
all.

However, since the serialized data are corrupt, I see no need to
"fix" that.
 [2021-06-17 11:36 UTC] j7ur8 at qq dot com
-Status: Feedback +Status: Open
 [2021-06-17 11:36 UTC] j7ur8 at qq dot com 
So, unserialize doesn't guarantee __wakeup executed before __destruct?

The string sequence i provided is illegal(at least not so correct):

    ` O:1:"C":1:{s:1:"c";O:1:"D":0:{};N;} ` // https://3v4l.org/M9bQJ

the orders is:

>PHP Notice:  unserialize(): Error at offset 31 of 35 bytes in 
>Notice: unserialize(): Error at offset 31 of 35 bytes in 
>C::__destruct
>D::__get(b)
>D::__wakeup
>D::__destruct

as we can see, __get executed before __wakeup. And if the sequence is correct:

    ` O:1:"C":1:{s:1:"c";O:1:"D":0:{}} `   // https://3v4l.org/UMahR

the orders is :
>D::__wakeup
>C::__destruct
>D::__get(b)
>D::__destruct


So, This is reasonable?
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Fri Nov 22 03:01:27 2024 UTC