php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #69111 Crash in SessionHandler::read()
Submitted: 2015-02-23 20:33 UTC Modified: 2016-01-15 07:29 UTC
From: nikic@php.net Assigned: yohgaki
Status: Closed Package: Session related
PHP Version: 5.5.22 OS:
Private report: No CVE-ID:
 [2015-02-23 20:33 UTC] nikic@php.net
Description:
------------
The script

<?php
$sh = new SessionHandler;
session_set_save_handler($sh);

$savePath = ini_get('session.save_path');
$sessionName = ini_get('session.name');

// session_start(); // Uncommenting this makes it not crash when reading the session (see below), but it will not return any data. 

$sh->open($savePath, $sessionName);
$sh->write("foo", "bar");
$sh->read($id);

Results in a crash (running with -n). Backtrace:

Program received signal SIGSEGV, Segmentation fault.
__strcmp_ssse3 () at ../sysdeps/i386/i686/multiarch/strcmp-ssse3.S:233
233	../sysdeps/i386/i686/multiarch/strcmp-ssse3.S: No such file or directory.
(gdb) bt
#0  __strcmp_ssse3 () at ../sysdeps/i386/i686/multiarch/strcmp-ssse3.S:233
#1  0x082ba633 in ps_files_open (data=0xb7fbfa58, key=0x0, tsrm_ls=0x894bd80)
    at /home/nikic/dev/php-5.5/ext/session/mod_files.c:127
#2  0x082bb146 in ps_read_files (mod_data=0x89eea24, key=0xb7fc0e7c "", 
    val=0xbfffbc9c, vallen=0xbfffbca4, tsrm_ls=0x894bd80)
    at /home/nikic/dev/php-5.5/ext/session/mod_files.c:362
#3  0x082acca3 in zim_SessionHandler_read (ht=1, return_value=0xb7fc0eb0, 
    return_value_ptr=0x0, this_ptr=0xb7fbdf48, return_value_used=0, 
    tsrm_ls=0x894bd80)
    at /home/nikic/dev/php-5.5/ext/session/mod_user_class.c:83
#4  0x08486b53 in zend_do_fcall_common_helper_SPEC (execute_data=0xb7fa216c, 
    tsrm_ls=0x894bd80) at /home/nikic/dev/php-5.5/Zend/zend_vm_execute.h:550
#5  0x084873e4 in ZEND_DO_FCALL_BY_NAME_SPEC_HANDLER (execute_data=0xb7fa216c, 
    tsrm_ls=0x894bd80) at /home/nikic/dev/php-5.5/Zend/zend_vm_execute.h:685
#6  0x084860ff in execute_ex (execute_data=0xb7fa216c, tsrm_ls=0x894bd80)
    at /home/nikic/dev/php-5.5/Zend/zend_vm_execute.h:363
#7  0x084861b4 in zend_execute (op_array=0xb7fbe7f4, tsrm_ls=0x894bd80)
    at /home/nikic/dev/php-5.5/Zend/zend_vm_execute.h:388
#8  0x0844ad81 in zend_execute_scripts (type=8, tsrm_ls=0x894bd80, retval=0x0, 
    file_count=3) at /home/nikic/dev/php-5.5/Zend/zend.c:1327
#9  0x083ad5f2 in php_execute_script (primary_file=0xbfffe0f0, 
    tsrm_ls=0x894bd80) at /home/nikic/dev/php-5.5/main/main.c:2506
#10 0x084f4783 in do_cli (argc=2, argv=0x894bce8, tsrm_ls=0x894bd80)

The issue is that PS(id) is NULL in the following line:

(gdb) f 2
#2  0x082bb146 in ps_read_files (mod_data=0x89eea24, key=0xb7fc0e7c "", 
    val=0xbfffbc9c, vallen=0xbfffbca4, tsrm_ls=0x894bd80)
    at /home/nikic/dev/php-5.5/ext/session/mod_files.c:362
362		ps_files_open(data, PS(id) TSRMLS_CC);

This is a regression from https://github.com/php/php-src/commit/16411586449c7562b840d6226f6ef55f567c35f3 which fixed bug #65475.



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-02-23 20:33 UTC] nikic@php.net
-Assigned To: +Assigned To: yohgaki
 [2015-02-23 20:38 UTC] nikic@php.net
Credit to http://www.reddit.com/user/Nicoon for finding this issue :)
 [2015-02-23 21:25 UTC] yohgaki@php.net
Since session_start() isn't called, data->lastkey isn't initialized in this case.
Session save handler is not supposed be called by user script, but it seems I should initialize session handler data by session_save_handler().

This requires additional save handler API for save handler data initialization.

In mean time, I can invade session module's save handlers (files, mm) without adding new save handler API, but 3rd party modules may crash like this.
 [2015-02-23 21:32 UTC] yohgaki@php.net
Rather than initializing data by save handler API. I may ask all save handler developers to call session module API to check the session state.

I'll fix this by using above method.
Comments are appreciated.
 [2015-04-24 06:41 UTC] yohgaki@php.net
I made a PoC patch for this.

https://github.com/php/php-src/pull/1248

It's not too complicated, but I would rather remove use of previously defined save handler functions as SessionHandler base class. It just makes things more complicated than needed already. I just don't think we should add more complication to it.
 [2016-01-15 07:27 UTC] yohgaki@php.net
Automatic comment on behalf of yohgaki
Revision: http://git.php.net/?p=php-src.git;a=commit;h=bfb9307b2d679a91e138fd876880470ece60942b
Log: Fixed bug #69111 (Crash in SessionHandler::read()). Made session save handler abuse much harder than before.
 [2016-01-15 07:27 UTC] yohgaki@php.net
-Status: Assigned +Status: Closed
 [2016-01-15 07:29 UTC] yohgaki@php.net
Used PS(session_status) to disable this kind of abuse. Fixed from PHP 5.6.
 [2016-01-29 10:26 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=25108babdbd17a8213c93c5139eda382c15f9aa4
Log: refix bug #69111, crash in 5.6 only
 [2016-01-29 18:59 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=80f7b0125875116fdbcdaf48cf8b1bbf93cb378c
Log: refix #69111 and one related test
 [2016-01-29 20:07 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6891e6abdf3c29caa465aefe80587de1db3a91ff
Log: Revert &quot;refix #69111 and one related test&quot;
 [2016-01-29 20:08 UTC] ab@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=ebcfe7618da2e1dfdb2cd691db8e3582e6310b0c
Log: Revert &quot;refix #69111 and one related test&quot;
 [2016-02-03 09:20 UTC] tyrael@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=a793b709086eed655bc98f933d838b8679b28920
Log: refix bug #69111, crash in 5.6 only
 [2016-07-20 11:33 UTC] davey@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6891e6abdf3c29caa465aefe80587de1db3a91ff
Log: Revert &quot;refix #69111 and one related test&quot;
 [2016-07-20 11:33 UTC] davey@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=ebcfe7618da2e1dfdb2cd691db8e3582e6310b0c
Log: Revert &quot;refix #69111 and one related test&quot;
 [2016-07-20 11:33 UTC] davey@php.net
Automatic comment on behalf of ab
Revision: http://git.php.net/?p=php-src.git;a=commit;h=80f7b0125875116fdbcdaf48cf8b1bbf93cb378c
Log: refix #69111 and one related test
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Wed Feb 22 06:01:36 2017 UTC