Fix UAF issues with PCRE after request shutdown

There are two related issues, each tested.

First problem:
What happens is that on the CLI SAPI we have a per-request pcre cache,
and on there the request shutdown for the pcre module happens prior to
the remaining live object destruction. So when the SPL object wants to
clean up the regular expression object it gets a use-after-free.

Second problem:
Very similarly, the non-persistent resources are destroyed after request
shutdown, so on the CLI SAPI the pcre request cache is already gone, but
if a userspace stream references a regex in the pcre cache, this breaks.

Two things that come immediately to mind:
  -  We could fix it by no longer treating the CLI SAPI special and just use
     the same lifecycle as the module. This simplifies the pcre module code
     a bit too. I wonder why we even have the separation in the first place.
     The downside here is that we're using more the system allocator
     than Zend's allocator for cache entries.
  -  We could modify the shutdown code to not remove regular expressions
     with a refcount>0 and modify php_pcre_pce_decref code such that it
     becomes php_pcre_pce_decref's job to clean up when the refcount
     becomes 0 during shutdown. However, this gets nasty quickly.

I chose the first solution here as it should be reliable and simple.

Closes phpGH-15064.

Author: ndossche

NEWS

@@ -9,6 +9,9 @@ PHP                                                                        NEWS
 - Opcache:
   . Fixed bug GH-15657 (Segmentation fault in dasm_x86.h). (nielsdos)
 
+- PCRE:
+  . Fix UAF issues with PCRE after request shutdown. (nielsdos)
+
 - SOAP:
   . Fixed bug #73182 (PHP SOAPClient does not support stream context HTTP
     headers in array form). (nielsdos)

UPGRADING.INTERNALS

@@ -358,6 +358,9 @@ PHP 8.4 INTERNALS UPGRADE NOTES
    - pcre_get_compiled_regex_cache_ex() now provides an option to collect extra
      options (from modifiers used in the expression, for example), and calls
      pcre2_set_compile_extra_options() with those options.
+   - Removed per-request cache, the cache is now always per process or
+     per thread depending on whether you use NTS or ZTS.
+     This was removed due to fundamental ordering issues between destructors.
 
  g. ext/standard
    - Added the php_base64_encode_ex() API with flag parameters, value can be

ext/opcache/ZendAccelerator.c

@@ -2611,10 +2611,6 @@ static void accel_reset_pcre_cache(void)
 {
 	Bucket *p;
 
-	if (PCRE_G(per_request_cache)) {
-		return;
-	}
-
 	ZEND_HASH_MAP_FOREACH_BUCKET(&PCRE_G(pcre_cache), p) {
 		/* Remove PCRE cache entries with inconsistent keys */
 		if (zend_accel_in_shm(p->key)) {

ext/pcre/php_pcre.c

@@ -92,7 +92,7 @@ static MUTEX_T pcre_mt = NULL;
 
 ZEND_TLS HashTable char_tables;
 
-static void free_subpats_table(zend_string **subpat_names, uint32_t num_subpats, bool persistent);
+static void free_subpats_table(zend_string **subpat_names, uint32_t num_subpats);
 
 static void php_pcre_free_char_table(zval *data)
 {/*{{{*/
@@ -168,25 +168,13 @@ static void php_free_pcre_cache(zval *data) /* {{{ */
 	pcre_cache_entry *pce = (pcre_cache_entry *) Z_PTR_P(data);
 	if (!pce) return;
 	if (pce->subpats_table) {
-		free_subpats_table(pce->subpats_table, pce->capture_count + 1, true);
+		free_subpats_table(pce->subpats_table, pce->capture_count + 1);
 	}
 	pcre2_code_free(pce->re);
 	free(pce);
 }
 /* }}} */
 
-static void php_efree_pcre_cache(zval *data) /* {{{ */
-{
-	pcre_cache_entry *pce = (pcre_cache_entry *) Z_PTR_P(data);
-	if (!pce) return;
-	if (pce->subpats_table) {
-		free_subpats_table(pce->subpats_table, pce->capture_count + 1, false);
-	}
-	pcre2_code_free(pce->re);
-	efree(pce);
-}
-/* }}} */
-
 static void *php_pcre_malloc(PCRE2_SIZE size, void *data)
 {
 	return pemalloc(size, 1);
@@ -303,12 +291,7 @@ static PHP_GINIT_FUNCTION(pcre) /* {{{ */
 {
 	php_pcre_mutex_alloc();
 
-	/* If we're on the CLI SAPI, there will only be one request, so we don't need the
-	 * cache to survive after RSHUTDOWN. */
-	pcre_globals->per_request_cache = strcmp(sapi_module.name, "cli") == 0;
-	if (!pcre_globals->per_request_cache) {
-		zend_hash_init(&pcre_globals->pcre_cache, 0, NULL, php_free_pcre_cache, 1);
-	}
+	zend_hash_init(&pcre_globals->pcre_cache, 0, NULL, php_free_pcre_cache, 1);
 
 	pcre_globals->backtrack_limit = 0;
 	pcre_globals->recursion_limit = 0;
@@ -326,9 +309,7 @@ static PHP_GINIT_FUNCTION(pcre) /* {{{ */
 
 static PHP_GSHUTDOWN_FUNCTION(pcre) /* {{{ */
 {
-	if (!pcre_globals->per_request_cache) {
-		zend_hash_destroy(&pcre_globals->pcre_cache);
-	}
+	zend_hash_destroy(&pcre_globals->pcre_cache);
 
 	php_pcre_shutdown_pcre2();
 	zend_hash_destroy(&char_tables);
@@ -491,10 +472,6 @@ static PHP_RINIT_FUNCTION(pcre)
 		return FAILURE;
 	}
 
-	if (PCRE_G(per_request_cache)) {
-		zend_hash_init(&PCRE_G(pcre_cache), 0, NULL, php_efree_pcre_cache, 0);
-	}
-
 	return SUCCESS;
 }
 /* }}} */
@@ -504,10 +481,6 @@ static PHP_RSHUTDOWN_FUNCTION(pcre)
 	pcre2_general_context_free(PCRE_G(gctx_zmm));
 	PCRE_G(gctx_zmm) = NULL;
 
-	if (PCRE_G(per_request_cache)) {
-		zend_hash_destroy(&PCRE_G(pcre_cache));
-	}
-
 	zval_ptr_dtor(&PCRE_G(unmatched_null_pair));
 	zval_ptr_dtor(&PCRE_G(unmatched_empty_pair));
 	ZVAL_UNDEF(&PCRE_G(unmatched_null_pair));
@@ -530,18 +503,18 @@ static int pcre_clean_cache(zval *data, void *arg)
 }
 /* }}} */
 
-static void free_subpats_table(zend_string **subpat_names, uint32_t num_subpats, bool persistent) {
+static void free_subpats_table(zend_string **subpat_names, uint32_t num_subpats) {
 	uint32_t i;
 	for (i = 0; i < num_subpats; i++) {
 		if (subpat_names[i]) {
-			zend_string_release_ex(subpat_names[i], persistent);
+			zend_string_release_ex(subpat_names[i], true);
 		}
 	}
-	pefree(subpat_names, persistent);
+	pefree(subpat_names, true);
 }
 
 /* {{{ static make_subpats_table */
-static zend_string **make_subpats_table(uint32_t name_cnt, pcre_cache_entry *pce, bool persistent)
+static zend_string **make_subpats_table(uint32_t name_cnt, pcre_cache_entry *pce)
 {
 	uint32_t num_subpats = pce->capture_count + 1;
 	uint32_t name_size, ni = 0;
@@ -556,7 +529,7 @@ static zend_string **make_subpats_table(uint32_t name_cnt, pcre_cache_entry *pce
 		return NULL;
 	}
 
-	subpat_names = pecalloc(num_subpats, sizeof(zend_string *), persistent);
+	subpat_names = pecalloc(num_subpats, sizeof(zend_string *), true);
 	while (ni++ < name_cnt) {
 		unsigned short name_idx = 0x100 * (unsigned char)name_table[0] + (unsigned char)name_table[1];
 		const char *name = name_table + 2;
@@ -566,10 +539,8 @@ static zend_string **make_subpats_table(uint32_t name_cnt, pcre_cache_entry *pce
 		 * Although we will be storing them in user-exposed arrays, they cannot cause problems
 		 * because they only live in this thread and the last reference is deleted on shutdown
 		 * instead of by user code. */
-		subpat_names[name_idx] = zend_string_init(name, strlen(name), persistent);
-		if (persistent) {
-			GC_MAKE_PERSISTENT_LOCAL(subpat_names[name_idx]);
-		}
+		subpat_names[name_idx] = zend_string_init(name, strlen(name), true);
+		GC_MAKE_PERSISTENT_LOCAL(subpat_names[name_idx]);
 		name_table += name_size;
 	}
 	return subpat_names;
@@ -871,7 +842,7 @@ PHPAPI pcre_cache_entry* pcre_get_compiled_regex_cache_ex(zend_string *regex, bo
 
 	/* Compute and cache the subpattern table to avoid computing it again over and over. */
 	if (name_count > 0) {
-		new_entry.subpats_table = make_subpats_table(name_count, &new_entry, !PCRE_G(per_request_cache));
+		new_entry.subpats_table = make_subpats_table(name_count, &new_entry);
 		if (!new_entry.subpats_table) {
 			if (key != regex) {
 				zend_string_release_ex(key, false);
@@ -892,7 +863,7 @@ PHPAPI pcre_cache_entry* pcre_get_compiled_regex_cache_ex(zend_string *regex, bo
 	 * as hash keys especually for this table.
 	 * See bug #63180
 	 */
-	if (!(GC_FLAGS(key) & IS_STR_PERMANENT) && !PCRE_G(per_request_cache)) {
+	if (!(GC_FLAGS(key) & IS_STR_PERMANENT)) {
 		zend_string *str = zend_string_init(ZSTR_VAL(key), ZSTR_LEN(key), 1);
 		GC_MAKE_PERSISTENT_LOCAL(str);
 
@@ -963,18 +934,18 @@ PHPAPI void php_pcre_free_match_data(pcre2_match_data *match_data)
 	}
 }/*}}}*/
 
-static void init_unmatched_null_pair(void) {
+static void init_unmatched_null_pair(zval *pair) {
 	zval val1, val2;
 	ZVAL_NULL(&val1);
 	ZVAL_LONG(&val2, -1);
-	ZVAL_ARR(&PCRE_G(unmatched_null_pair), zend_new_pair(&val1, &val2));
+	ZVAL_ARR(pair, zend_new_pair(&val1, &val2));
 }
 
-static void init_unmatched_empty_pair(void) {
+static void init_unmatched_empty_pair(zval *pair) {
 	zval val1, val2;
 	ZVAL_EMPTY_STRING(&val1);
 	ZVAL_LONG(&val2, -1);
-	ZVAL_ARR(&PCRE_G(unmatched_empty_pair), zend_new_pair(&val1, &val2));
+	ZVAL_ARR(pair, zend_new_pair(&val1, &val2));
 }
 
 static zend_always_inline void populate_match_value_str(
@@ -1020,15 +991,29 @@ static inline void add_offset_pair(
 	/* Add (match, offset) to the return value */
 	if (PCRE2_UNSET == start_offset) {
 		if (unmatched_as_null) {
-			if (Z_ISUNDEF(PCRE_G(unmatched_null_pair))) {
-				init_unmatched_null_pair();
-			}
-			ZVAL_COPY(&match_pair, &PCRE_G(unmatched_null_pair));
+			do {
+				if (Z_ISUNDEF(PCRE_G(unmatched_null_pair))) {
+					if (UNEXPECTED(EG(flags) & EG_FLAGS_IN_SHUTDOWN)) {
+						init_unmatched_null_pair(&match_pair);
+						break;
+					} else {
+						init_unmatched_null_pair(&PCRE_G(unmatched_null_pair));
+					}
+				}
+				ZVAL_COPY(&match_pair, &PCRE_G(unmatched_null_pair));
+			} while (0);
 		} else {
-			if (Z_ISUNDEF(PCRE_G(unmatched_empty_pair))) {
-				init_unmatched_empty_pair();
-			}
-			ZVAL_COPY(&match_pair, &PCRE_G(unmatched_empty_pair));
+			do {
+				if (Z_ISUNDEF(PCRE_G(unmatched_empty_pair))) {
+					if (UNEXPECTED(EG(flags) & EG_FLAGS_IN_SHUTDOWN)) {
+						init_unmatched_empty_pair(&match_pair);
+						break;
+					} else {
+						init_unmatched_empty_pair(&PCRE_G(unmatched_empty_pair));
+					}
+				}
+				ZVAL_COPY(&match_pair, &PCRE_G(unmatched_empty_pair));
+			} while (0);
 		}
 	} else {
 		zval val1, val2;

ext/pcre/php_pcre.h

@@ -78,7 +78,6 @@ ZEND_BEGIN_MODULE_GLOBALS(pcre)
 #ifdef HAVE_PCRE_JIT_SUPPORT
 	bool jit;
 #endif
-	bool per_request_cache;
 	php_pcre_error_code error_code;
 	/* Used for unmatched subpatterns in OFFSET_CAPTURE mode */
 	zval unmatched_null_pair;

ext/pcre/tests/gh15205_1.phpt

@@ -0,0 +1,12 @@
+--TEST--
+GH-15205: UAF when destroying RegexIterator after pcre request shutdown
+--CREDITS--
+YuanchengJiang
+--FILE--
+<?php
+$array = new ArrayIterator(array('a', array('b', 'c')));
+$regex = [new RegexIterator($array, '/Array/')];
+echo "Done\n";
+?>
+--EXPECT--
+Done

ext/pcre/tests/gh15205_2.phpt

@@ -0,0 +1,32 @@
+--TEST--
+GH-15205: UAF when destroying stream after pcre request shutdown
+--CREDITS--
+nicolaslegland
+--FILE--
+<?php
+// Prime cache
+preg_replace('/pattern/', 'replace', 'subject');
+
+class wrapper
+{
+	public function stream_open($path, $mode, $options, &$opened_path)
+	{
+		return true;
+	}
+
+	public function stream_close()
+	{
+		echo "Close\n";
+		preg_replace('/pattern/', 'replace', 'subject');
+		preg_match('/(4)?(2)?\d/', '23456', $matches, PREG_OFFSET_CAPTURE | PREG_UNMATCHED_AS_NULL);
+		preg_match('/(4)?(2)?\d/', '23456', $matches, PREG_OFFSET_CAPTURE);
+	}
+
+	public $context;
+}
+
+stream_wrapper_register('wrapper', 'wrapper');
+$handle = fopen('wrapper://', 'rb');
+?>
+--EXPECT--
+Close