Re: com php-src: Fixed bug #69111 (Crash in SessionHandler::read()). Made session save handler abuse much harder than before.: NEWS
ext/session/mod_user.c ext/session/mod_user_class.c ext/session/session.c ext/session/tests/bug55688.phpt ext/session/tests/bug60634.phpt
ext/session/tests/bug60634_error_1.phpt ext/session/tests/bug60634_error_5.phpt ext/session/tests/bug67972.phpt ext/session/tests/bug69111.phpt
ext/session/tests/sessionhandler_open_00

Hi all, On Fri, Jan 15, 2016 at 3:50 PM, Yasuo Ohgaki <yohgaki@php.net> wrote: > Commit: bfb9307b2d679a91e138fd876880470ece60942b > Author: Yasuo Ohgaki <yohgaki@php.net> Fri, 15 Jan 2016 13:47:45 +0900 > Parents: d7f8d9e3a9babf0e4f0c1a5590e1feb5e69bd84a > Branches: PHP-5.6 PHP-7.0 master > > Link: > http://git.php.net/?p=php-src.git;a=commitdiff;h=bfb9307b2d679a91e138fd876880470ece60942b > > Log: > Fixed bug #69111 (Crash in SessionHandler::read()). > Made session save handler abuse much harder than before. > > Bugs: > https://bugs.php.net/69111 > > Changed paths: > M NEWS > M ext/session/mod_user.c > M ext/session/mod_user_class.c > M ext/session/session.c > M ext/session/tests/bug55688.phpt > M ext/session/tests/bug60634.phpt > M ext/session/tests/bug60634_error_1.phpt > M ext/session/tests/bug60634_error_5.phpt > M ext/session/tests/bug67972.phpt > A ext/session/tests/bug69111.phpt > M ext/session/tests/sessionhandler_open_001.phpt I recently fixed crash bug by session save handler abuse. It became much harder to abuse save handler callbacks, but it's not complete because current session module does not check bailout from user handler. For instance, http://lxr.php.net/xref/PHP_MASTER/ext/session/session.c#2128 s_read() calls user script read handler, but mod_user*.c does not use zend_try() for it. http://lxr.php.net/xref/PHP_MASTER/ext/session/mod_user.c#131 User handler uses zend_try few places in mod_user*.c, but it does not use zend_try where it is required. The reason why zend_try is not used is save handler code should not touch session module internal data structures. i.e. Variable cleanups are needed in some places and it results in needless save handler calls on error in limited case. I may use zend_try in session.c rather than mod_user*.c. This way, I can do all cleanup tasks required up on errors. Downside is a little overhead of zend_try macros. zend_try is not required for native save handlers at all. We have 4 options - Don't care. Error shouldn't happen under normal circumstances. (Leave as it is now) - Use zend_try in session.c (Unneeded overhead for native handlers) - Use zend_try in mod_user*.c, if EG(bailout) is changed, call zend_bailout() from session.c. - Use flag if session.c should use zend_try or not. (Code will look ugly, but it works) I would like to hear which option is preferred, not preferred. I don't have preference. If nobody cares, I'll leave it as is. Thanks. -- Yasuo Ohgaki yohgaki@ohgaki.net