Skip to content

Commit 8763c60

Browse files
committed
Fix bug #72681 - consume data even if we're not storing them
1 parent 448c9be commit 8763c60

File tree

2 files changed

+30
-3
lines changed

2 files changed

+30
-3
lines changed

Diff for: ext/session/session.c

+14-3
Original file line numberDiff line numberDiff line change
@@ -924,11 +924,13 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */
924924
int namelen;
925925
int has_value;
926926
php_unserialize_data_t var_hash;
927+
int skip = 0;
927928

928929
PHP_VAR_UNSERIALIZE_INIT(var_hash);
929930

930931
for (p = val; p < endptr; ) {
931932
zval **tmp;
933+
skip = 0;
932934
namelen = ((unsigned char)(*p)) & (~PS_BIN_UNDEF);
933935

934936
if (namelen < 0 || namelen > PS_BIN_MAX || (p + namelen) >= endptr) {
@@ -944,22 +946,25 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */
944946

945947
if (zend_hash_find(&EG(symbol_table), name, namelen + 1, (void **) &tmp) == SUCCESS) {
946948
if ((Z_TYPE_PP(tmp) == IS_ARRAY && Z_ARRVAL_PP(tmp) == &EG(symbol_table)) || *tmp == PS(http_session_vars)) {
947-
efree(name);
948-
continue;
949+
skip = 1;
949950
}
950951
}
951952

952953
if (has_value) {
953954
ALLOC_INIT_ZVAL(current);
954955
if (php_var_unserialize(&current, (const unsigned char **) &p, (const unsigned char *) endptr, &var_hash TSRMLS_CC)) {
956+
if (!skip) {
955957
php_set_session_var(name, namelen, current, &var_hash TSRMLS_CC);
958+
}
956959
} else {
957960
PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
958961
return FAILURE;
959962
}
960963
var_push_dtor_no_addref(&var_hash, &current);
961964
}
965+
if (!skip) {
962966
PS_ADD_VARL(name, namelen);
967+
}
963968
efree(name);
964969
}
965970

@@ -1016,6 +1021,7 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
10161021
int namelen;
10171022
int has_value;
10181023
php_unserialize_data_t var_hash;
1024+
int skip = 0;
10191025

10201026
PHP_VAR_UNSERIALIZE_INIT(var_hash);
10211027

@@ -1024,6 +1030,7 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
10241030
while (p < endptr) {
10251031
zval **tmp;
10261032
q = p;
1033+
skip = 0;
10271034
while (*q != PS_DELIMITER) {
10281035
if (++q >= endptr) goto break_outer_loop;
10291036
}
@@ -1040,14 +1047,16 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
10401047

10411048
if (zend_hash_find(&EG(symbol_table), name, namelen + 1, (void **) &tmp) == SUCCESS) {
10421049
if ((Z_TYPE_PP(tmp) == IS_ARRAY && Z_ARRVAL_PP(tmp) == &EG(symbol_table)) || *tmp == PS(http_session_vars)) {
1043-
goto skip;
1050+
skip = 1;
10441051
}
10451052
}
10461053

10471054
if (has_value) {
10481055
ALLOC_INIT_ZVAL(current);
10491056
if (php_var_unserialize(&current, (const unsigned char **) &q, (const unsigned char *) endptr, &var_hash TSRMLS_CC)) {
1057+
if (!skip) {
10501058
php_set_session_var(name, namelen, current, &var_hash TSRMLS_CC);
1059+
}
10511060
} else {
10521061
var_push_dtor_no_addref(&var_hash, &current);
10531062
efree(name);
@@ -1056,7 +1065,9 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
10561065
}
10571066
var_push_dtor_no_addref(&var_hash, &current);
10581067
}
1068+
if (!skip) {
10591069
PS_ADD_VARL(name, namelen);
1070+
}
10601071
skip:
10611072
efree(name);
10621073

Diff for: ext/session/tests/bug72681.phpt

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
Bug #72681: PHP Session Data Injection Vulnerability
3+
--SKIPIF--
4+
<?php include('skipif.inc'); ?>
5+
--FILE--
6+
<?php
7+
ini_set('session.serialize_handler', 'php');
8+
session_start();
9+
$_SESSION['_SESSION'] = 'ryat|O:8:"stdClass":0:{}';
10+
session_write_close();
11+
session_start();
12+
var_dump($_SESSION);
13+
?>
14+
--EXPECT--
15+
array(0) {
16+
}

0 commit comments

Comments
 (0)