Saner numeric string handling for string offsets

Girgias · Girgias · commit c4a006b8fd7d · 2020-07-02T12:46:58.000+02:00
diff --git a/Zend/tests/bug24773.phpt b/Zend/tests/bug24773.phpt
@@ -6,7 +6,7 @@ Bug #24773 (unset() of integers treated as arrays causes a crash)
     unset($array["lvl1"]["lvl2"]["b"]);
 ?>
 --EXPECTF--
-Fatal error: Uncaught Error: Cannot use string offset as an array in %s:%d
+Fatal error: Uncaught TypeError: Illegal offset type in %s:%d
 Stack trace:
 #0 {main}
   thrown in %s on line %d
diff --git a/Zend/tests/bug31098.phpt b/Zend/tests/bug31098.phpt
@@ -17,16 +17,28 @@ var_dump(isset($a['b']));
 
 $simpleString = "Bogus String Text";
 echo isset($simpleString->wrong)?"bug\n":"ok\n";
-echo isset($simpleString["wrong"])?"bug\n":"ok\n";
+try {
+    echo isset($simpleString["wrong"])?"bug\n":"ok\n";
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 echo isset($simpleString[-20])?"bug\n":"ok\n";
 echo isset($simpleString[0])?"ok\n":"bug\n";
 echo isset($simpleString["0"])?"ok\n":"bug\n";
 echo isset($simpleString["16"])?"ok\n":"bug\n";
 echo isset($simpleString["17"])?"bug\n":"ok\n";
 echo $simpleString->wrong === null?"ok\n":"bug\n";
-echo $simpleString["wrong"] === "B"?"ok\n":"bug\n";
+try {
+    echo $simpleString["wrong"] === "B"?"ok\n":"bug\n";
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 echo $simpleString["0"] === "B"?"ok\n":"bug\n";
-$simpleString["wrong"] = "f";
+try {
+    $simpleString["wrong"] = "f";
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 echo $simpleString["0"] === "f"?"ok\n":"bug\n";
 ?>
 --EXPECTF--
@@ -46,10 +58,7 @@ ok
 
 Warning: Attempt to read property 'wrong' on string in %s on line %d
 ok
-
-Warning: Illegal string offset 'wrong' in %s on line %d
-ok
+Illegal offset type
 ok
-
-Warning: Illegal string offset 'wrong' in %s on line %d
+Illegal offset type
 ok
diff --git a/Zend/tests/bug53432.phpt b/Zend/tests/bug53432.phpt
@@ -16,7 +16,11 @@ var_dump($str[-1] = 'a');
 var_dump($str);
 
 $str = '';
-var_dump($str['foo'] = 'a');
+try {
+    var_dump($str['foo'] = 'a');
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 var_dump($str);
 
 $str = '';
@@ -53,9 +57,7 @@ string(6) "     a"
 Warning: Illegal string offset:  -1 in %s on line %d
 NULL
 string(0) ""
-
-Warning: Illegal string offset 'foo' in %s on line %d
-string(1) "a"
+Illegal offset type
 string(1) "a"
 Error: [] operator not supported for strings
 string(0) ""
diff --git a/Zend/tests/bug64578.phpt b/Zend/tests/bug64578.phpt
@@ -5,10 +5,10 @@ Bug #64578 (debug_backtrace in set_error_handler corrupts zend heap: segfault)
 
 set_error_handler(function($no, $err) { var_dump($err); });
 
-function x($s) { $s['a'] = 1; };
+function x($s) { $s['2a'] = 1; };
 $y = '1';
 x($y);
 print_r($y);
 --EXPECT--
-string(25) "Illegal string offset 'a'"
+string(26) "Illegal string offset '2a'"
 1
diff --git a/Zend/tests/bug73792.phpt b/Zend/tests/bug73792.phpt
@@ -4,15 +4,15 @@ Bug #73792 (invalid foreach loop hangs script)
 <?php
 $a = 'aaa';
 
-foreach ($a['bbb'] as &$value) {
+foreach ($a['2bbb'] as &$value) {
     echo 'loop';
 }
 
 unset($value);
 echo 'done';
 ?>
 --EXPECTF--
-Warning: Illegal string offset 'bbb' in %sbug73792.php on line 4
+Warning: Illegal string offset '2bbb' in %sbug73792.php on line 4
 
 Fatal error: Uncaught Error: Cannot iterate on string offsets by reference in %sbug73792.php:4
 Stack trace:
diff --git a/Zend/tests/bug76534.phpt b/Zend/tests/bug76534.phpt
@@ -7,10 +7,10 @@ set_error_handler(function ($severity, $message, $file, $line) {
 });
 
 $x = "foo";
-$y = &$x["bar"];
+$y = &$x["2bar"];
 ?>
 --EXPECTF--
-Fatal error: Uncaught Exception: Illegal string offset 'bar' in %sbug76534.php:%d
+Fatal error: Uncaught Exception: Illegal string offset '2bar' in %sbug76534.php:%d
 Stack trace:
 #0 %sbug76534.php(%d): {closure}(2, 'Illegal string ...', '%s', %d)
 #1 {main}
diff --git a/Zend/tests/const_dereference_002.phpt b/Zend/tests/const_dereference_002.phpt
@@ -6,12 +6,12 @@ error_reporting(E_ALL);
 
 var_dump("foobar"[3]);
 var_dump("foobar"[2][0]);
-var_dump("foobar"["foo"]["bar"]);
+var_dump("foobar"["0foo"]["0bar"]);
 --EXPECTF--
 string(1) "b"
 string(1) "o"
 
-Warning: Illegal string offset 'foo' in %sconst_dereference_002.php on line %d
+Warning: Illegal string offset '0foo' in %sconst_dereference_002.php on line %d
 
-Warning: Illegal string offset 'bar' in %sconst_dereference_002.php on line %d
+Warning: Illegal string offset '0bar' in %sconst_dereference_002.php on line %d
 string(1) "f"
diff --git a/Zend/tests/indexing_001.phpt b/Zend/tests/indexing_001.phpt
@@ -75,19 +75,13 @@ array(1) {
   }
 }
 
-Warning: Illegal string offset 'foo' in %s on line %d
-
 Warning: Array to string conversion in %s on line %d
-
-Warning: Only the first byte will be assigned to the string offset in %s on line %d
-string(1) "A"
-
-Warning: Illegal string offset 'foo' in %s on line %d
+Illegal offset type
+string(0) ""
 
 Warning: Array to string conversion in %s on line %d
-
-Warning: Only the first byte will be assigned to the string offset in %s on line %d
-string(1) "A"
+Illegal offset type
+string(1) " "
 Cannot use a scalar value as an array
 float(0.1)
 array(1) {
diff --git a/Zend/tests/numeric_strings/string_offset.phpt b/Zend/tests/numeric_strings/string_offset.phpt
@@ -30,19 +30,19 @@ echo "Done\n";
 --EXPECTF--
 string(1) "l"
 
-Warning: Illegal string offset '7.5' in %s on line 6
+Warning: String offset cast occurred in %s on line 6
 string(1) "l"
 string(1) "l"
 
-Warning: Illegal string offset '  7.5' in %s on line 8
+Warning: String offset cast occurred in %s on line 8
 string(1) "l"
 string(1) "l"
 
-Warning: Illegal string offset '  7.5  ' in %s on line 10
+Warning: String offset cast occurred in %s on line 10
 string(1) "l"
 string(1) "l"
 
-Warning: Illegal string offset '7.5  ' in %s on line 12
+Warning: String offset cast occurred in %s on line 12
 string(1) "l"
 
 Warning: Illegal string offset '7str' in %s on line 13
diff --git a/Zend/tests/offset_assign.phpt b/Zend/tests/offset_assign.phpt
@@ -1,14 +1,14 @@
 --TEST--
-Crash on $x['x']['y'] += 1 when $x is string
+Crash on $x['2x']['y'] += 1 when $x is string
 --FILE--
 <?php
 $x = "a";
-$x['x']['y'] += 1;
+$x['2x']['y'] += 1;
 
 echo "Done\n";
 ?>
 --EXPECTF--
-Warning: Illegal string offset 'x' in %soffset_assign.php on line %d
+Warning: Illegal string offset '2x' in %soffset_assign.php on line %d
 
 Fatal error: Uncaught Error: Cannot use string offset as an array in %soffset_assign.php:%d
 Stack trace:
diff --git a/Zend/tests/offset_string.phpt b/Zend/tests/offset_string.phpt
@@ -8,7 +8,11 @@ $str = "Sitting on a corner all alone, staring from the bottom of his soul";
 var_dump($str[1]);
 var_dump($str[0.0836]);
 var_dump($str[NULL]);
-var_dump($str["run away"]);
+try {
+    var_dump($str["run away"]);
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 var_dump($str["13"]);
 var_dump($str["14.5"]);
 var_dump($str["15 and then some"]);
@@ -47,12 +51,10 @@ string(1) "S"
 
 Warning: String offset cast occurred in %s on line %d
 string(1) "S"
-
-Warning: Illegal string offset 'run away' in %s on line %d
-string(1) "S"
+Illegal offset type
 string(1) "c"
 
-Warning: Illegal string offset '14.5' in %s on line %d
+Warning: String offset cast occurred in %s on line %d
 string(1) "o"
 
 Warning: Illegal string offset '15 and then some' in %s on line %d
diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c
@@ -1331,13 +1331,28 @@ static zend_never_inline zend_long zend_check_string_offset(zval *dim, int type
 	if (UNEXPECTED(Z_TYPE_P(dim) != IS_LONG)) {
 		switch(Z_TYPE_P(dim)) {
 			case IS_STRING:
-				if (IS_LONG == is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), NULL, NULL, false)) {
-					break;
+			{
+				/* allow errors in string offset for BC reasons */
+				zend_uchar numeric_string_type = is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), &offset, NULL, true);
+				if (IS_LONG == numeric_string_type) {
+					/* emit Illegal string warning on leading numerical string */
+					if (0 == is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), NULL, NULL, false)
+							&& type != BP_VAR_UNSET) {
+						zend_error(E_WARNING, "Illegal string offset '%s'", Z_STRVAL_P(dim));
+					}
+					return offset;
 				}
-				if (type != BP_VAR_UNSET) {
-					zend_error(E_WARNING, "Illegal string offset '%s'", Z_STRVAL_P(dim));
+				if (IS_DOUBLE == numeric_string_type) {
+					if (0 == is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), NULL, NULL, false)) {
+						zend_error(E_WARNING, "Illegal string offset '%s'", Z_STRVAL_P(dim));
+					} else {
+						zend_error(E_WARNING, "String offset cast occurred");
+					}
+					break;
 				}
+				zend_illegal_offset();
 				break;
+			}
 			case IS_UNDEF:
 				ZVAL_UNDEFINED_OP2();
 			case IS_DOUBLE:
@@ -2259,17 +2274,33 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 try_string_offset:
 		if (UNEXPECTED(Z_TYPE_P(dim) != IS_LONG)) {
 			switch (Z_TYPE_P(dim)) {
-				/* case IS_LONG: */
 				case IS_STRING:
-					if (IS_LONG == is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), NULL, NULL, false)) {
+				{
+					/* allow errors in string offset for BC reasons */
+					zend_uchar numeric_string_type = is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), &offset, NULL, true);
+					if (IS_LONG == numeric_string_type) {
+						/* emit Illegal string warning on leading numerical string */
+						if (0 == is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), NULL, NULL, false)) {
+							zend_error(E_WARNING, "Illegal string offset '%s'", Z_STRVAL_P(dim));
+						}
+						goto out;
+					}
+					if (IS_DOUBLE == numeric_string_type) {
+						if (type != BP_VAR_IS &&
+							0 == is_numeric_string(Z_STRVAL_P(dim), Z_STRLEN_P(dim), NULL, NULL, false)) {
+							zend_error(E_WARNING, "Illegal string offset '%s'", Z_STRVAL_P(dim));
+						} else if (type != BP_VAR_IS) {
+							zend_error(E_WARNING, "String offset cast occurred");
+						}
 						break;
 					}
 					if (type == BP_VAR_IS) {
 						ZVAL_NULL(result);
 						return;
 					}
-					zend_error(E_WARNING, "Illegal string offset '%s'", Z_STRVAL_P(dim));
+					zend_illegal_offset();
 					break;
+				}
 				case IS_UNDEF:
 					ZVAL_UNDEFINED_OP2();
 				case IS_DOUBLE:
@@ -2292,6 +2323,7 @@ static zend_always_inline void zend_fetch_dimension_address_read(zval *result, z
 		} else {
 			offset = Z_LVAL_P(dim);
 		}
+		out:
 
 		if (UNEXPECTED(Z_STRLEN_P(container) < ((offset < 0) ? -(size_t)offset : ((size_t)offset + 1)))) {
 			if (type != BP_VAR_IS) {
diff --git a/tests/lang/bug19943.phpt b/tests/lang/bug19943.phpt
@@ -5,11 +5,11 @@ Bug #19943 (memleaks)
     $ar = array();
     for ($count = 0; $count < 10; $count++) {
         $ar[$count]        = "$count";
-        @$ar[$count]['idx'] = "$count";
+        @$ar[$count]['0idx'] = "$count";
     }
 
     for ($count = 0; $count < 10; $count++) {
-        echo $ar[$count]." -- ".@$ar[$count]['idx']."\n";
+        echo $ar[$count]." -- ".@$ar[$count]['0idx']."\n";
     }
     $a = "0123456789";
     $a[9] = $a[0];
diff --git a/tests/lang/bug29566.phpt b/tests/lang/bug29566.phpt
@@ -7,10 +7,10 @@ $var="This is a string";
 $dummy="";
 unset($dummy);
 
-foreach($var['nosuchkey'] as $v) {
+foreach($var['0.5nosuchkey'] as $v) {
 }
 ?>
 --EXPECTF--
-Warning: Illegal string offset 'nosuchkey' in %sbug29566.php on line %d
+Warning: Illegal string offset '0.5nosuchkey' in %sbug29566.php on line %d
 
 Warning: foreach() argument must be of type array|object, string given in %sbug29566.php on line %d
diff --git a/tests/strings/offsets_chaining_5.phpt b/tests/strings/offsets_chaining_5.phpt
@@ -6,20 +6,20 @@ $array = array('expected_array' => "foobar");
 var_dump(isset($array['expected_array']));
 var_dump($array['expected_array']);
 var_dump(isset($array['expected_array']['foo']));
-var_dump($array['expected_array']['foo']);
+var_dump($array['expected_array']['0foo']);
 var_dump(isset($array['expected_array']['foo']['bar']));
-var_dump($array['expected_array']['foo']['bar']);
+var_dump($array['expected_array']['0foo']['0bar']);
 ?>
 --EXPECTF--
 bool(true)
 string(6) "foobar"
 bool(false)
 
-Warning: Illegal string offset 'foo' in %soffsets_chaining_5.php on line %d
+Warning: Illegal string offset '0foo' in %soffsets_chaining_5.php on line %d
 string(1) "f"
 bool(false)
 
-Warning: Illegal string offset 'foo' in %soffsets_chaining_5.php on line %d
+Warning: Illegal string offset '0foo' in %soffsets_chaining_5.php on line %d
 
-Warning: Illegal string offset 'bar' in %soffsets_chaining_5.php on line %d
+Warning: Illegal string offset '0bar' in %soffsets_chaining_5.php on line %d
 string(1) "f"
diff --git a/tests/strings/offsets_general.phpt b/tests/strings/offsets_general.phpt
@@ -9,17 +9,19 @@ var_dump($string[0]);
 var_dump($string[1]);
 var_dump(isset($string[0]));
 var_dump(isset($string[0][0]));
-var_dump($string["foo"]);
+try {
+    var_dump($string["foo"]);
+} catch (\TypeError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
 var_dump(isset($string["foo"]["bar"]));
 
 ?>
---EXPECTF--
+--EXPECT--
 string(1) "B"
 string(1) "f"
 string(1) "o"
 bool(true)
 bool(true)
-
-Warning: Illegal string offset 'foo' in %s line %d
-string(1) "f"
+Illegal offset type
 bool(false)
