CVE-2016-7125: Improper Neutralization of Special Elements in Output Used by a Downstream Component ('Injection') (CWE-74)
value: ext/session/session.c in PHP before 5.6.25 and 7.x before 7.0.10 skips invalid session names in a way that triggers incorrect parsing, which allows remote attackers to inject arbitrary-type session data by leveraging control of a session name, as demonstrated by object injection.
CWE description: The product constructs all or part of a command, data structure, or record using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify how it is parsed or interpreted when it is sent to a downstream component.
CVSS v3 base score: 7.5
Published: 2016-09-12T01:59Z
Repository: https://github.com/php/php-src
Language: C
Files changed: ext/session/session.c, ext/session/tests/bug72681.phpt
Commit date: 2016-08-16 22:54:42 -0700
Commit message: Fix bug #72681 - consume data even if we're not storing them

Diff: diff --git a/ext/session/session.c b/ext/session/session.c
index c668bb7b2a2..b2d02361dfd 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -922,15 +922,17 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */
 	const char *endptr = val + vallen;
 	zval *current;
 	int namelen;
 	int has_value;
 	php_unserialize_data_t var_hash;
+	int skip = 0;
 
 	PHP_VAR_UNSERIALIZE_INIT(var_hash);
 
 	for (p = val; p < endptr; ) {
 		zval **tmp;
+		skip = 0;
 		namelen = ((unsigned char)(*p)) & (~PS_BIN_UNDEF);
 
 		if (namelen < 0 || namelen > PS_BIN_MAX || (p + namelen) >= endptr) {
 			PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 			return FAILURE;
@@ -942,26 +944,29 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */
 
 		p += namelen + 1;
 
 		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)) {
-				efree(name);
-				continue;
+				skip = 1;
 			}
 		}
 
 		if (has_value) {
 			ALLOC_INIT_ZVAL(current);
 			if (php_var_unserialize(&current, (const unsigned char **) &p, (const unsigned char *) endptr, &var_hash TSRMLS_CC)) {
-				php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
+				if (!skip) {
+					php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
+				}
 			} else {
 				PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 				return FAILURE;
 			}
 			var_push_dtor_no_addref(&var_hash, &current);
 		}
-		PS_ADD_VARL(name, namelen);
+		if (!skip) {
+			PS_ADD_VARL(name, namelen);
+		}
 		efree(name);
 	}
 
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 
@@ -1014,18 +1019,20 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
 	const char *endptr = val + vallen;
 	zval *current;
 	int namelen;
 	int has_value;
 	php_unserialize_data_t var_hash;
+	int skip = 0;
 
 	PHP_VAR_UNSERIALIZE_INIT(var_hash);
 
 	p = val;
 
 	while (p < endptr) {
 		zval **tmp;
 		q = p;
+		skip = 0;
 		while (*q != PS_DELIMITER) {
 			if (++q >= endptr) goto break_outer_loop;
 		}
 		if (p[0] == PS_UNDEF_MARKER) {
 			p++;
@@ -1038,27 +1045,31 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
 		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 = 1;
 			}
 		}
 
 		if (has_value) {
 			ALLOC_INIT_ZVAL(current);
 			if (php_var_unserialize(&current, (const unsigned char **) &q, (const unsigned char *) endptr, &var_hash TSRMLS_CC)) {
-				php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
+				if (!skip) {
+					php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
+				}
 			} else {
 				var_push_dtor_no_addref(&var_hash, &current);
 				efree(name);
 				PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 				return FAILURE;
 			}
 			var_push_dtor_no_addref(&var_hash, &current);
 		}
-		PS_ADD_VARL(name, namelen);
+		if (!skip) {
+			PS_ADD_VARL(name, namelen);
+		}
 skip:
 		efree(name);
 
 		p = q;
 	}
diff --git a/ext/session/tests/bug72681.phpt b/ext/session/tests/bug72681.phpt
new file mode 100644
index 00000000000..ca38b07fc90
--- /dev/null
+++ b/ext/session/tests/bug72681.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Bug #72681: PHP Session Data Injection Vulnerability
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+ini_set('session.serialize_handler', 'php');
+session_start();
+$_SESSION['_SESSION'] = 'ryat|O:8:"stdClass":0:{}';
+session_write_close();
+session_start();
+var_dump($_SESSION);
+?>
+--EXPECT--
+array(0) {
+}

Vulnerable code: 				efree(name);
				continue;
				php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
		PS_ADD_VARL(name, namelen);
				goto skip;
				php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
		PS_ADD_VARL(name, namelen);
Fixed code: 	int skip = 0;
		skip = 0;
				skip = 1;
				if (!skip) {
					php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
				}
		if (!skip) {
			PS_ADD_VARL(name, namelen);
		}
	int skip = 0;
		skip = 0;
				skip = 1;
				if (!skip) {
					php_set_session_var(name, namelen, current, &var_hash  TSRMLS_CC);
				}
		if (!skip) {
			PS_ADD_VARL(name, namelen);
		}
--TEST--
Bug #72681: PHP Session Data Injection Vulnerability
--SKIPIF--
<?php include('skipif.inc'); ?>
--FILE--
<?php
ini_set('session.serialize_handler', 'php');
session_start();
$_SESSION['_SESSION'] = 'ryat|O:8:"stdClass":0:{}';
session_write_close();
session_start();
var_dump($_SESSION);
?>
--EXPECT--
array(0) {
}