Fixed bug #71972 (Cyclic references causing session_start(): Failed to decode session object)

Author: laruence

NEWS

@@ -14,6 +14,10 @@ PHP                                                                        NEWS
   . Fixed bug #71062 (pg_convert() doesn't accept ISO 8601 for datatype
     timestamp). (denver at timothy dot io)
 
+- Session:
+  . Fixed bug #71972 (Cyclic references causing session_start(): Failed to
+    decode session object). (Laruence)
+
 - SQLite3:
   . Fixed bug #68849 (bindValue is not using the right data type). (Anatol)
 

ext/session/session.c

@@ -498,7 +498,6 @@ static void php_session_gc(void) /* {{{ */
 	}
 } /* }}} */
 
-
 static void php_session_initialize(void) /* {{{ */
 {
 	zend_string *val = NULL;
@@ -613,6 +612,22 @@ static void php_session_save_current_state(int write) /* {{{ */
 }
 /* }}} */
 
+static void php_session_normalize_vars() /* {{{ */
+{
+	PS_ENCODE_VARS;
+
+	IF_SESSION_VARS() {
+		PS_ENCODE_LOOP(
+			if (Z_TYPE_P(struc) == IS_PTR) {
+				zval *zv = (zval *)Z_PTR_P(struc);
+				ZVAL_COPY_VALUE(struc, zv);
+				ZVAL_UNDEF(zv);
+			}
+		);
+	}
+}
+/* }}} */
+
 /* *************************
    * INI Settings/Handlers *
    ************************* */
@@ -944,7 +959,6 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */
 {
 	const char *p;
 	const char *endptr = val + vallen;
-	zval current;
 	int has_value;
 	int namelen;
 	zend_string *name;
@@ -967,28 +981,32 @@ PS_SERIALIZER_DECODE_FUNC(php_binary) /* {{{ */
 		p += namelen + 1;
 
 		if ((tmp = zend_hash_find(&EG(symbol_table), name))) {
-			if ((Z_TYPE_P(tmp) == IS_ARRAY && Z_ARRVAL_P(tmp) == &EG(symbol_table)) || tmp == &PS(http_session_vars)) {
+			if ((Z_TYPE_P(tmp) == IS_ARRAY &&
+				Z_ARRVAL_P(tmp) == &EG(symbol_table)) || tmp == &PS(http_session_vars)) {
 				zend_string_release(name);
 				continue;
 			}
 		}
 
 		if (has_value) {
-			ZVAL_UNDEF(&current);
-			if (php_var_unserialize(&current, (const unsigned char **) &p, (const unsigned char *) endptr, &var_hash)) {
-				zval *zv = php_set_session_var(name, &current, &var_hash );
-				var_replace(&var_hash, &current, zv);
+			zval *current, rv;
+			current = var_tmp_var(&var_hash);
+			if (php_var_unserialize(current, (const unsigned char **) &p, (const unsigned char *) endptr, &var_hash)) {
+				ZVAL_PTR(&rv, current);
+				php_set_session_var(name, &rv, &var_hash );
 			} else {
-				zval_ptr_dtor(&current);
 				zend_string_release(name);
+				php_session_normalize_vars();
 				PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 				return FAILURE;
 			}
+		} else {
+			PS_ADD_VARL(name);
 		}
-		PS_ADD_VARL(name);
 		zend_string_release(name);
 	}
 
+	php_session_normalize_vars();
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 
 	return SUCCESS;
@@ -1033,10 +1051,9 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
 {
 	const char *p, *q;
 	const char *endptr = val + vallen;
-	zval current;
-	int has_value;
 	ptrdiff_t namelen;
 	zend_string *name;
+	int has_value, retval = SUCCESS;
 	php_unserialize_data_t var_hash;
 
 	PHP_VAR_UNSERIALIZE_INIT(var_hash);
@@ -1061,34 +1078,37 @@ PS_SERIALIZER_DECODE_FUNC(php) /* {{{ */
 		q++;
 
 		if ((tmp = zend_hash_find(&EG(symbol_table), name))) {
-			if ((Z_TYPE_P(tmp) == IS_ARRAY && Z_ARRVAL_P(tmp) == &EG(symbol_table)) || tmp == &PS(http_session_vars)) {
+			if ((Z_TYPE_P(tmp) == IS_ARRAY &&
+				Z_ARRVAL_P(tmp) == &EG(symbol_table)) || tmp == &PS(http_session_vars)) {
 				goto skip;
 			}
 		}
 
 		if (has_value) {
-			ZVAL_UNDEF(&current);
-			if (php_var_unserialize(&current, (const unsigned char **) &q, (const unsigned char *) endptr, &var_hash)) {
-				zval *zv = php_set_session_var(name, &current, &var_hash);
-				var_replace(&var_hash, &current, zv);
+			zval *current, rv;
+			current = var_tmp_var(&var_hash);
+			if (php_var_unserialize(current, (const unsigned char **)&q, (const unsigned char *)endptr, &var_hash)) {
+				ZVAL_PTR(&rv, current);
+				php_set_session_var(name, &rv, &var_hash);
 			} else {
-				zval_ptr_dtor(&current);
-				PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 				zend_string_release(name);
-				return FAILURE;
+				retval = FAILURE;
+				goto break_outer_loop;
 			}
+		} else {
+			PS_ADD_VARL(name);
 		}
-		PS_ADD_VARL(name);
 skip:
 		zend_string_release(name);
 
 		p = q;
 	}
 break_outer_loop:
+	php_session_normalize_vars();
 
 	PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 
-	return SUCCESS;
+	return retval;
 }
 /* }}} */
 

ext/session/tests/bug71972.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Bug #71972 (Cyclic references causing session_start(): Failed to decode session object)
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--INI--
+session.save_handler=files
+--FILE--
+<?php
+ob_start();
+session_start();
+
+$_SESSION['boogie'] = 1;
+
+$_SESSION['obj1'] = new stdClass();
+for ( $x=2; $x < 20; $x++) {
+	cyclic_ref($x);
+}
+
+function cyclic_ref($num) {
+	$_SESSION['obj'.$num] = new stdClass();
+	$_SESSION['obj'.$num]->test = new stdClass();//NOTE: No bug if try commenting out this too.
+	$_SESSION['obj'.$num]->obj1 = $_SESSION['obj1'];
+}
+
+var_dump(session_decode(session_encode()) == $_SESSION);
+?>
+--EXPECT--
+bool(true)