]> granicus.if.org Git - php/commitdiff
Fix bug #72681 - consume data even if we're not storing them
authorStanislav Malyshev <stas@php.net>
Wed, 3 Aug 2016 07:30:12 +0000 (00:30 -0700)
committerStanislav Malyshev <stas@php.net>
Wed, 17 Aug 2016 05:54:42 +0000 (22:54 -0700)
ext/session/session.c
ext/session/tests/bug72681.phpt [new file with mode: 0644]

index c668bb7b2a27c99a0795e825f3f7e94e256fab9d..b2d02361dfd139a2bddf402d96d2b819ba27eb77 100644 (file)
@@ -924,11 +924,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) {
@@ -944,22 +946,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(&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);
        }
 
@@ -1016,6 +1021,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);
 
@@ -1024,6 +1030,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;
                }
@@ -1040,14 +1047,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(&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);
@@ -1056,7 +1065,9 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
                        }
                        var_push_dtor_no_addref(&var_hash, &current);
                }
-               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 (file)
index 0000000..ca38b07
--- /dev/null
@@ -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) {
+}