Fix phpGH-20194: null offset deprecation not emitted for writes (php#20238)

Based on a patch from @ndossche

Author: Girgias

Zend/Optimizer/sccp.c

@@ -1106,6 +1106,11 @@ static void sccp_visit_instr(scdf_ctx *scdf, zend_op *opline, zend_ssa_op *ssa_o
 
 			if (op2) {
 				SKIP_IF_TOP(op2);
+				if (Z_TYPE_P(op2) == IS_NULL) {
+					/* Emits deprecation at run-time. */
+					SET_RESULT_BOT(result);
+					return;
+				}
 			}
 
 			/* We want to avoid keeping around intermediate arrays for each SSA variable in the

Zend/Optimizer/zend_inference.c

@@ -5252,7 +5252,7 @@ ZEND_API bool zend_may_throw_ex(const zend_op *opline, const zend_ssa_op *ssa_op
 		case ZEND_INIT_ARRAY:
 			return (opline->op2_type != IS_UNUSED) && (t2 & (MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
 		case ZEND_ADD_ARRAY_ELEMENT:
-			return (opline->op2_type == IS_UNUSED) || (t2 & (MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
+			return (opline->op2_type == IS_UNUSED) || (t2 & (MAY_BE_NULL|MAY_BE_ARRAY|MAY_BE_OBJECT|MAY_BE_RESOURCE));
 		case ZEND_STRLEN:
 			return (t1 & MAY_BE_ANY) != MAY_BE_STRING;
 		case ZEND_COUNT:

Zend/tests/constexpr/constant_expressions_dynamic.phpt

@@ -48,6 +48,8 @@ var_dump(
 Warning: A non-numeric value encountered in %s on line %d
 
 Deprecated: Implicit conversion from float 3.14 to int loses precision in %s on line %d
+
+Deprecated: Using null as an array offset is deprecated, use an empty string instead in %s on line %d
 int(3)
 string(4) "1foo"
 bool(false)

Zend/tests/offsets/gh20194.phpt

@@ -0,0 +1,15 @@
+--TEST--
+GH-20194: Using null as an array offset does not emit deprecation when resolved at compile time
+--FILE--
+<?php
+
+$a = [null => 1];
+
+echo $a[null];
+
+?>
+--EXPECTF--
+Deprecated: Using null as an array offset is deprecated, use an empty string instead in %s on line %d
+
+Deprecated: Using null as an array offset is deprecated, use an empty string instead in %s on line %d
+1

Zend/tests/offsets/null_offset_dep_promoted.phpt

@@ -0,0 +1,17 @@
+--TEST--
+Do not leak when promoting null offset deprecation
+--FILE--
+<?php
+
+set_error_handler(function ($errno, $errstr) {
+    throw new Exception($errstr);
+});
+
+try {
+	$a = ['foo' => 'bar', null => new stdClass];
+} catch (Throwable $e) {
+	echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+?>
+--EXPECT--
+Exception: Using null as an array offset is deprecated, use an empty string instead

Zend/tests/offsets/null_offset_unset_via_error_handler.phpt

@@ -0,0 +1,19 @@
+--TEST--
+No UAF on null offset deprecation with unset value
+--FILE--
+<?php
+set_error_handler(function ($errno, $errstr) {
+    var_dump($errstr);
+    global $a;
+    unset($a);
+});
+
+$a = new stdClass;
+$b = [0, null => $a];
+
+echo "\nSuccess\n";
+?>
+--EXPECTF--
+string(72) "Using null as an array offset is deprecated, use an empty string instead"
+
+Success

Zend/zend_API.c

@@ -2255,9 +2255,6 @@ ZEND_API zend_result array_set_zval_key(HashTable *ht, zval *key, zval *value) /
 		case IS_STRING:
 			result = zend_symtable_update(ht, Z_STR_P(key), value);
 			break;
-		case IS_NULL:
-			result = zend_hash_update(ht, ZSTR_EMPTY_ALLOC(), value);
-			break;
 		case IS_RESOURCE:
 			zend_use_resource_as_offset(key);
 			result = zend_hash_index_update(ht, Z_RES_HANDLE_P(key), value);
@@ -2274,6 +2271,13 @@ ZEND_API zend_result array_set_zval_key(HashTable *ht, zval *key, zval *value) /
 		case IS_DOUBLE:
 			result = zend_hash_index_update(ht, zend_dval_to_lval_safe(Z_DVAL_P(key)), value);
 			break;
+		case IS_NULL:
+			zend_error(E_DEPRECATED, "Using null as an array offset is deprecated, use an empty string instead");
+			if (UNEXPECTED(EG(exception))) {
+				return FAILURE;
+			}
+			result = zend_hash_update(ht, ZSTR_EMPTY_ALLOC(), value);
+			break;
 		default:
 			zend_illegal_container_offset(ZSTR_KNOWN(ZEND_STR_ARRAY), key, BP_VAR_W);
 			result = NULL;

Zend/zend_compile.c

@@ -10175,9 +10175,7 @@ static bool zend_try_ct_eval_array(zval *result, zend_ast *ast) /* {{{ */
 					zend_long lval = zend_dval_to_lval_silent(Z_DVAL_P(key));
 					/* Incompatible float will generate an error, leave this to run-time */
 					if (!zend_is_long_compatible(Z_DVAL_P(key), lval)) {
-						zval_ptr_dtor_nogc(value);
-						zval_ptr_dtor(result);
-						return 0;
+						goto fail;
 					}
 					zend_hash_index_update(Z_ARRVAL_P(result), lval, value);
 					break;
@@ -10189,13 +10187,14 @@ static bool zend_try_ct_eval_array(zval *result, zend_ast *ast) /* {{{ */
 					zend_hash_index_update(Z_ARRVAL_P(result), 1, value);
 					break;
 				case IS_NULL:
-					zend_hash_update(Z_ARRVAL_P(result), ZSTR_EMPTY_ALLOC(), value);
-					break;
+					/* Null key will generate a warning at run-time. */
+					goto fail;
 				default:
 					zend_error_noreturn(E_COMPILE_ERROR, "Illegal offset type");
 					break;
 			}
 		} else if (!zend_hash_next_index_insert(Z_ARRVAL_P(result), value)) {
+fail:
 			zval_ptr_dtor_nogc(value);
 			zval_ptr_dtor(result);
 			return 0;

Zend/zend_vm_def.h

@@ -6278,7 +6278,21 @@ ZEND_VM_C_LABEL(num_index):
 		} else if ((OP2_TYPE & (IS_VAR|IS_CV)) && EXPECTED(Z_TYPE_P(offset) == IS_REFERENCE)) {
 			offset = Z_REFVAL_P(offset);
 			ZEND_VM_C_GOTO(add_again);
-		} else if (Z_TYPE_P(offset) == IS_NULL) {
+		} else if (UNEXPECTED(Z_TYPE_P(offset) == IS_NULL)) {
+			zval tmp;
+			if (OP1_TYPE == IS_CV || OP1_TYPE == IS_VAR) {
+				ZVAL_COPY(&tmp, expr_ptr);
+			}
+			zend_error(E_DEPRECATED, "Using null as an array offset is deprecated, use an empty string instead");
+			if (OP1_TYPE == IS_CV || OP1_TYPE == IS_VAR) {
+				/* A userland error handler can do funky things to the expression, so reset it */
+				zval_ptr_dtor(expr_ptr);
+				ZVAL_COPY_VALUE(expr_ptr, &tmp);
+			}
+			if (UNEXPECTED(EG(exception))) {
+				zval_ptr_dtor_nogc(expr_ptr);
+				HANDLE_EXCEPTION();
+			}
 			str = ZSTR_EMPTY_ALLOC();
 			ZEND_VM_C_GOTO(str_index);
 		} else if (Z_TYPE_P(offset) == IS_DOUBLE) {