ext/spl: Fix ArrayObject unserialize validation for invalid iterator classes#22090
Conversation
| } | ||
|
|
||
| if (!instanceof_function(ce, zend_ce_iterator)) { | ||
| if (!instanceof_function(ce, spl_ce_ArrayIterator)) { |
There was a problem hiding this comment.
This is more restrictive than the current code. But then I have no idea if it even makes sens to attempt to use any iterator. But in this case the error message needs to be fixed.
There was a problem hiding this comment.
Thank you.
Please check the following code.
$arrayObject = new ArrayObject(
[
4 => 0.0,
1 => true
],
0,
"GlobIterator"
);
var_dump( $arrayObject);Output:
Fatal error: Uncaught TypeError: ArrayObject::__construct(): Argument #3 ($iteratorClass) must be a class name derived from ArrayIterator, GlobIterator given in /Users/arshid/Downloads/php-src/z.php:20
Stack trace:
#0 /Users/arshid/Downloads/php-src/z.php(20): ArrayObject->__construct(Array, 0, 'GlobIterator')
#1 {main}
thrown in /Users/arshid/Downloads/php-src/z.php on line 20https://github.com/php/php-src/blob/master/Zend/zend_API.c#L1001
|
I'm not the most familiar with behaviour relating to unserialisation, maybe @TimWolla has an opinion? |
TimWolla
left a comment
TimWolla
left a comment
There was a problem hiding this comment.
I'm not the most familiar with behaviour relating to unserialisation
This is not really an unserialization question, but rather a question of what “legal inputs are”. As demonstrated in #22090 (comment), the constructor rejected non-ArrayIterators, thus deserialization should as well. This PR seems correct to me.
| @@ -0,0 +1,19 @@ | |||
| --TEST-- | |||
| GH-22047: ArrayObject invalid iterator class in serialized payload | |||
There was a problem hiding this comment.
The CREDITS section was missing (which I told the reporter we'll add). Added here: 61e679d We should get in the habit of doing this, I added it to my personal test template a while ago.
5 participants
#22047