Merge branch 'PHP-8.4'

* PHP-8.4:
  Fix phpGH-16590: UAF in session_encode()
  Fix various memory leaks on error conditions in openssl_x509_parse()

Author: ndossche

ext/openssl/openssl.c

@@ -2153,15 +2153,15 @@ PHP_FUNCTION(openssl_x509_parse)
 	/* Can return NULL on error or memory allocation failure */
 	if (!bn_serial) {
 		php_openssl_store_errors();
-		RETURN_FALSE;
+		goto err;
 	}
 
 	hex_serial = BN_bn2hex(bn_serial);
 	BN_free(bn_serial);
 	/* Can return NULL on error or memory allocation failure */
 	if (!hex_serial) {
 		php_openssl_store_errors();
-		RETURN_FALSE;
+		goto err;
 	}
 
 	str_serial = i2s_ASN1_INTEGER(NULL, asn1_serial);
@@ -2233,19 +2233,15 @@ PHP_FUNCTION(openssl_x509_parse)
 		bio_out = BIO_new(BIO_s_mem());
 		if (bio_out == NULL) {
 			php_openssl_store_errors();
-			RETURN_FALSE;
+			goto err_subitem;
 		}
 		if (nid == NID_subject_alt_name) {
 			if (openssl_x509v3_subjectAltName(bio_out, extension) == 0) {
 				BIO_get_mem_ptr(bio_out, &bio_buf);
 				add_assoc_stringl(&subitem, extname, bio_buf->data, bio_buf->length);
 			} else {
-				zend_array_destroy(Z_ARR_P(return_value));
 				BIO_free(bio_out);
-				if (cert_str) {
-					X509_free(cert);
-				}
-				RETURN_FALSE;
+				goto err_subitem;
 			}
 		}
 		else if (X509V3_EXT_print(bio_out, extension, 0, 0)) {
@@ -2260,6 +2256,16 @@ PHP_FUNCTION(openssl_x509_parse)
 	if (cert_str) {
 		X509_free(cert);
 	}
+	return;
+
+err_subitem:
+	zval_ptr_dtor(&subitem);
+err:
+	zend_array_destroy(Z_ARR_P(return_value));
+	if (cert_str) {
+		X509_free(cert);
+	}
+	RETURN_FALSE;
 }
 /* }}} */
 

ext/session/php_session.h

@@ -291,8 +291,13 @@ PHPAPI zend_result php_session_reset_id(void);
 	zend_ulong num_key;													\
 	zval *struc;
 
+/* Do not use a return statement in `code` because that may leak memory.
+ * Break out of the loop instead. */
 #define PS_ENCODE_LOOP(code) do {									\
-	HashTable *_ht = Z_ARRVAL_P(Z_REFVAL(PS(http_session_vars)));	\
+	zval _zv;														\
+	/* protect against user interference */							\
+	ZVAL_COPY(&_zv, Z_REFVAL(PS(http_session_vars)));				\
+	HashTable *_ht = Z_ARRVAL(_zv);									\
 	ZEND_HASH_FOREACH_KEY(_ht, num_key, key) {						\
 		if (key == NULL) {											\
 			php_error_docref(NULL, E_WARNING,						\
@@ -303,6 +308,7 @@ PHPAPI zend_result php_session_reset_id(void);
 			code;		 											\
 		} 															\
 	} ZEND_HASH_FOREACH_END();										\
+	zval_ptr_dtor(&_zv);											\
 } while(0)
 
 PHPAPI ZEND_EXTERN_MODULE_GLOBALS(ps)

ext/session/session.c

@@ -1079,6 +1079,7 @@ PS_SERIALIZER_ENCODE_FUNC(php) /* {{{ */
 {
 	smart_str buf = {0};
 	php_serialize_data_t var_hash;
+	bool fail = false;
 	PS_ENCODE_VARS;
 
 	PHP_VAR_SERIALIZE_INIT(var_hash);
@@ -1088,12 +1089,17 @@ PS_SERIALIZER_ENCODE_FUNC(php) /* {{{ */
 		if (memchr(ZSTR_VAL(key), PS_DELIMITER, ZSTR_LEN(key))) {
 			PHP_VAR_SERIALIZE_DESTROY(var_hash);
 			smart_str_free(&buf);
-			return NULL;
+			fail = true;
+			break;
 		}
 		smart_str_appendc(&buf, PS_DELIMITER);
 		php_var_serialize(&buf, struc, &var_hash);
 	);
 
+	if (fail) {
+		return NULL;
+	}
+
 	smart_str_0(&buf);
 
 	PHP_VAR_SERIALIZE_DESTROY(var_hash);

ext/session/tests/gh16590.phpt

@@ -0,0 +1,36 @@
+--TEST--
+GH-16590 (UAF in session_encode())
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--INI--
+session.use_cookies=0
+session.cache_limiter=
+session.serialize_handler=php
+session.save_handler=files
+--FILE--
+<?php
+
+class C {
+    function __serialize() {
+        $_SESSION = [];
+        return [];
+    }
+}
+
+session_start();
+
+$_SESSION['Lz'] = new C;
+for ($i = 0; $i < 2; $i++) {
+    $_SESSION[$i] = $i;
+}
+
+var_dump(session_encode());
+
+?>
+--EXPECTF--
+Warning: session_encode(): Skipping numeric key 0 in %s on line %d
+
+Warning: session_encode(): Skipping numeric key 1 in %s on line %d
+string(15) "Lz|O:1:"C":0:{}"