Crash in PHP7 when Unserializing

[msgpack]

Crash in PHP7 when Unserializing

[Sean-Der]

Thanks for the great report (all the information I need is here) @caleblloyd !

This crash doesn't happen in PHP 5? I should be able to look at this tonight, we read/write properties differently in the PHP 7 fork (you hit a segfault in new code) so not surprised looking at that BT.

thanks!

[caleblloyd]

No problem! Correct, when I run the same code in PHP5.6 with the php-msgpack from apt there is no crash. Thanks for looking into this!

[Sean-Der]

Bad news @caleblloyd I wasn't able to reproduce this :(

I am using msgpack at commit 034ddac305ffa004f7f44dc79fe81dbfe92833b1 and PHP at 4dee0c4a14a00f353d982a470694f907aa7f5239

PHP 7.0.1-dev (cli) (built: Nov 19 2015 07:08:58) ( NTS DEBUG )
Copyright (c) 1997-2015 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2015 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2015, by Zend Technologies
    with Xdebug v2.4.0rc1-dev, Copyright (c) 2002-2015, by Derick Rethans

and my script looks like

<?php

$curFile = file_get_contents(__DIR__ . '/msgpack.txt');
$unpacked = msgpack_unpack($curFile);
var_dump($unpacked);

[caleblloyd]

It's got a serialized class from the Yii (version 1) framework, I had to unserialize it from within Yii to get the error or else it returns Null.

I'll work up a test script for you that reproduces, should have that posted by Monday.

[caleblloyd]

@Sean-Der this bug is a doozy. I can only get it to happen when run from php-fpm, inside of the Yii controller. I've created a github repository and published an image to dockerhub that should help you recreate:

https://github.com/caleblloyd/php7-msgpack-bug

Instructions are in the repository README.md, let me know if you have any trouble getting it running. Hope it helps you find the bug!

[caleblloyd]

Just tried again with PHP7 RC8, same problem. Updated the Docker image to debug with to RC8. Took a stab at debugging the extension but I'm no expert with C. Anyone pointers on what might be causing this?

[Sean-Der]

Hey @caleblloyd !

I really appreciate all the work you have done getting the docker stuff together, that is really awesome! Sorry to leave you hanging, I am always one step behind on everything :(

I will reproduce this tonight and keep you updated! Hopefully an easy fix. Are you interested in extension development? If so more than happy to answer any questions! Always fun to learn new things.

[caleblloyd]

I got one step closer - making this change to the zend_read_property call now causes an exception instead of a segmentation fault: https://github.com/caleblloyd/msgpack-php/commit/a40313655620c6e41062592e0ac06caa5fc85603. Docker image has been updated.

The Exception is:

Property "CUrlRule._e" is not defined.

But if you look at the class that is being deserialized, you can see that:

class CUrlRule extends CBaseUrlRule
https://github.com/yiisoft/yii/blob/1.1.16/framework/web/CUrlManager.php#L564

abstract class CBaseUrlRule extends CComponent
https://github.com/yiisoft/yii/blob/1.1.16/framework/web/CUrlManager.php#L526

class CComponent contains the private property $_e
https://github.com/yiisoft/yii/blob/1.1.16/framework/base/CComponent.php#L89

According to http://www.phpinternalsbook.com/classes_objects/simple_classes.html, zend_read_property's first parameter is the object scope, and to read a private property the scope must be set to the proper class. I think that the code here is probably referencing the CUrlRule class as the scope reading the property instead of the CComponent class:
https://github.com/caleblloyd/msgpack-php/blob/php7/msgpack_unpack.c#L615

[Sean-Der]

Hey @caleblloyd, good news I figured out the issue (it is two fold)

1.) Incorrectly calling zend_read_property -- thanks for fixing that!

2.) In PHP 5.* we didn't go through the standard read/write for objects, but now we do. This is calling the frameworks __set / __get (which is throwing the exception you now see) https://github.com/yiisoft/yii/blob/master/framework/base/CComponent.php#L173

This is a weird bug, so working on tracking down the right fix now. Thanks for the patience.

[Sean-Der]

Hey @caleblloyd !

Would you mind trying out e0f2819, I also added a test to make sure we don't regress this again. If that fixes it for you, you can close the issue.

thanks for using msgpack :)

[caleblloyd]

@Sean-Der just tested it, the fix works great! Thanks a lot for your help with this. Is there a good book or online resource for learning PHP Internals? I'd like to learn more about it and contribute/develop extensions in the future

[Sean-Der]

Awesome :)

I hear a lot of good things about the PHP Internals Book a good place to get the basics down. However, what I end up doing is just copying what is in php-src. Use lxr if you are curious how a function works just search it!

If there are any other extensions that haven't been ported to PHP 7 I would love to help with them (a great place to learn) I just hack on the ones Etsy needs, but always looking for things to do. Oh and best of all, if you have a cool idea try to build it! Always available to review code/ideas.

thanks

[razvanphp]

It is the same problem I ended up in testing igbinary:
https://github.com/igbinary/igbinary7/blob/master/tests/igbinary_048.phpt

Thank you for the fix!