Backported from 5.6.25 by Remi. From 61156f0d68704df748b5cbf08c77582c208db8c9 Mon Sep 17 00:00:00 2001 From: Stanislav Malyshev Date: Wed, 3 Aug 2016 00:30:12 -0700 Subject: [PATCH] Fix bug #72681 - consume data even if we're not storing them --- ext/session/session.c | 25 ++++++++++++++++++------- ext/session/tests/bug72681.phpt | 16 ++++++++++++++++ 2 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 ext/session/tests/bug72681.phpt diff --git a/ext/session/session.c b/ext/session/session.c index c668bb7..b2d0236 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -923,11 +923,13 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */ 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) { @@ -943,22 +945,25 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */ 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(¤t, (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, ¤t); } - PS_ADD_VARL(name, namelen); + if (!skip) { + PS_ADD_VARL(name, namelen); + } efree(name); } @@ -1015,6 +1020,7 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ int namelen; int has_value; php_unserialize_data_t var_hash; + int skip = 0; PHP_VAR_UNSERIALIZE_INIT(var_hash); @@ -1023,6 +1029,7 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ while (p < endptr) { zval **tmp; q = p; + skip = 0; while (*q != PS_DELIMITER) { if (++q >= endptr) goto break_outer_loop; } @@ -1039,14 +1046,16 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ 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(¤t, (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, ¤t); efree(name); @@ -1055,7 +1064,9 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */ } var_push_dtor_no_addref(&var_hash, ¤t); } - PS_ADD_VARL(name, namelen); + if (!skip) { + PS_ADD_VARL(name, namelen); + } skip: efree(name); diff --git a/ext/session/tests/bug72681.phpt b/ext/session/tests/bug72681.phpt new file mode 100644 index 0000000..ca38b07 --- /dev/null +++ b/ext/session/tests/bug72681.phpt @@ -0,0 +1,16 @@ +--TEST-- +Bug #72681: PHP Session Data Injection Vulnerability +--SKIPIF-- + +--FILE-- + +--EXPECT-- +array(0) { +}