Fix crash after indirect modification of string by user error handler

Fixes oss-fuzz #39346
php · Nov 30, 2021 · df434f0 · df434f0
1 parent c103619
commit df434f0
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 3 deletions.
diff --git a/Zend/tests/str_offset_005.phpt b/Zend/tests/str_offset_005.phpt
@@ -0,0 +1,12 @@
+--TEST--
+string offset 005 indirect string modification by error handler
+--FILE--
+<?php
+set_error_handler(function(){$GLOBALS['a']=8;});
+$a='a';
+var_dump($a[$b]);
+var_dump($a);
+?>
+--EXPECT--
+string(1) "a"
+int(8)
diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -2361,6 +2361,7 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 		}
 	}
 	if (!is_list && EXPECTED(Z_TYPE_P(container) == IS_STRING)) {
+		zend_string *str = Z_STR_P(container);
 		zend_long offset;
 
 try_string_offset:
@@ -2386,13 +2387,33 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 					return;
 				}
 				case IS_UNDEF:
+					/* The string may be destroyed while throwing the notice.
+					 * Temporarily increase the refcount to detect this situation. */
+					if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE)) {
+						GC_ADDREF(str);
+					}
 					ZVAL_UNDEFINED_OP2();
+					if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE) && GC_DELREF(str) == 0) {
+						zend_string_release_ex(str, 0);
+						ZVAL_NULL(result);
+						return;
+					}
 				case IS_DOUBLE:
 				case IS_NULL:
 				case IS_FALSE:
 				case IS_TRUE:
 					if (type != BP_VAR_IS) {
+						/* The string may be destroyed while throwing the notice.
+						 * Temporarily increase the refcount to detect this situation. */
+						if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE)) {
+							GC_ADDREF(str);
+						}
 						zend_error(E_WARNING, "String offset cast occurred");
+						if (!(GC_FLAGS(str) & IS_ARRAY_IMMUTABLE) && GC_DELREF(str) == 0) {
+							zend_string_release_ex(str, 0);
+							ZVAL_NULL(result);
+							return;
+						}
 					}
 					break;
 				case IS_REFERENCE:
@@ -2410,7 +2431,7 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 		}
 		out:
 
-		if (UNEXPECTED(Z_STRLEN_P(container) < ((offset < 0) ? -(size_t)offset : ((size_t)offset + 1)))) {
+		if (UNEXPECTED(ZSTR_LEN(str) < ((offset < 0) ? -(size_t)offset : ((size_t)offset + 1)))) {
 			if (type != BP_VAR_IS) {
 				zend_error(E_WARNING, "Uninitialized string offset " ZEND_LONG_FMT, offset);
 				ZVAL_EMPTY_STRING(result);
@@ -2422,8 +2443,8 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 			zend_long real_offset;
 
 			real_offset = (UNEXPECTED(offset < 0)) /* Handle negative offset */
-				? (zend_long)Z_STRLEN_P(container) + offset : offset;
-			c = (zend_uchar)Z_STRVAL_P(container)[real_offset];
+				? (zend_long)ZSTR_LEN(str) + offset : offset;
+			c = (zend_uchar)ZSTR_VAL(str)[real_offset];
 
 			ZVAL_CHAR(result, c);
 		}