php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: j7ur8 at qq dot com
New email:
PHP Version: OS:

 

 [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: Sat Oct 12 23:01:27 2024 UTC