]> granicus.if.org Git - php/commitdiff
Fixed bug #78598
authorNikita Popov <nikita.ppv@gmail.com>
Tue, 4 Feb 2020 13:19:07 +0000 (14:19 +0100)
committerNikita Popov <nikita.ppv@gmail.com>
Tue, 7 Jul 2020 10:13:58 +0000 (12:13 +0200)
When performing an RW modification of an array offset, the undefined
offset warning may call an error handler / OB callback, which may
destroy the array we're supposed to change. Detect this by temporarily
incrementing the reference count. If we find that the array has been
modified/destroyed in the meantime, we do nothing -- the execution
model here would be that the modification has happened on the destroyed
version of the array.

NEWS
Zend/tests/bug70662.phpt
Zend/tests/bug78598.phpt [new file with mode: 0644]
Zend/tests/undef_index_to_exception.phpt [new file with mode: 0644]
Zend/zend_execute.c

diff --git a/NEWS b/NEWS
index fd9c57ba95e5c5c43e4f60d61837b8ed55c0ea19..7a0df9806a1f584e58f009ae1d5137227274fd55 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,8 @@ PHP                                                                        NEWS
     offset by reference). (Nikita)
   . Fixed bug #79792 (HT iterators not removed if empty array is destroyed).
     (Nikita)
+  . Fixed bug #78598 (Changing array during undef index RW error segfaults).
+    (Nikita)
 
 - Fileinfo:
   . Fixed bug #79756 (finfo_file crash (FILEINFO_MIME)). (cmb)
index 2bda8141bab4f04d0b2729beaaaa05c3908e186e..ab540c9d16e5fb11eff62ccda00841c6d1acb170 100644 (file)
@@ -14,5 +14,5 @@ var_dump($a);
 --EXPECT--
 array(1) {
   ["b"]=>
-  int(1)
+  int(2)
 }
diff --git a/Zend/tests/bug78598.phpt b/Zend/tests/bug78598.phpt
new file mode 100644 (file)
index 0000000..7e3559f
--- /dev/null
@@ -0,0 +1,31 @@
+--TEST--
+Bug #78598: Changing array during undef index RW error segfaults
+--FILE--
+<?php
+
+$my_var = null;
+set_error_handler(function() use(&$my_var) {
+    $my_var = 0;
+});
+
+$my_var[0] .= "xyz";
+var_dump($my_var);
+
+$my_var = null;
+$my_var[0][0][0] .= "xyz";
+var_dump($my_var);
+
+$my_var = null;
+$my_var["foo"] .= "xyz";
+var_dump($my_var);
+
+$my_var = null;
+$my_var["foo"]["bar"]["baz"] .= "xyz";
+var_dump($my_var);
+
+?>
+--EXPECT--
+int(0)
+int(0)
+int(0)
+int(0)
diff --git a/Zend/tests/undef_index_to_exception.phpt b/Zend/tests/undef_index_to_exception.phpt
new file mode 100644 (file)
index 0000000..d113608
--- /dev/null
@@ -0,0 +1,46 @@
+--TEST--
+Converting undefined index/offset notice to exception
+--FILE--
+<?php
+
+set_error_handler(function($_, $msg) {
+    throw new Exception($msg);
+});
+
+$test = [];
+try {
+    $test[0] .= "xyz";
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+var_dump($test);
+
+try {
+    $test["key"] .= "xyz";
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+var_dump($test);
+
+unset($test);
+try {
+    $GLOBALS["test"] .= "xyz";
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    var_dump($test);
+} catch (Exception $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Undefined offset: 0
+array(0) {
+}
+Undefined index: key
+array(0) {
+}
+Undefined index: test
+Undefined variable: test
index 89e6178019fc32df34d41e476cb3e85c0f386535..7b27c5a3a506070cec9f3a3b1e193de9bde2d030 100644 (file)
@@ -1958,6 +1958,44 @@ static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_undefined_index(const
        zend_error(E_NOTICE, "Undefined index: %s", ZSTR_VAL(offset));
 }
 
+static zend_never_inline ZEND_COLD int ZEND_FASTCALL zend_undefined_offset_write(
+               HashTable *ht, zend_long lval)
+{
+       /* The array may be destroyed while throwing the notice.
+        * Temporarily increase the refcount to detect this situation. */
+       if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
+               GC_ADDREF(ht);
+       }
+       zend_undefined_offset(lval);
+       if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) && !GC_DELREF(ht)) {
+               zend_array_destroy(ht);
+               return FAILURE;
+       }
+       if (EG(exception)) {
+               return FAILURE;
+       }
+       return SUCCESS;
+}
+
+static zend_never_inline ZEND_COLD int ZEND_FASTCALL zend_undefined_index_write(
+               HashTable *ht, zend_string *offset)
+{
+       /* The array may be destroyed while throwing the notice.
+        * Temporarily increase the refcount to detect this situation. */
+       if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE)) {
+               GC_ADDREF(ht);
+       }
+       zend_undefined_index(offset);
+       if (!(GC_FLAGS(ht) & IS_ARRAY_IMMUTABLE) && !GC_DELREF(ht)) {
+               zend_array_destroy(ht);
+               return FAILURE;
+       }
+       if (EG(exception)) {
+               return FAILURE;
+       }
+       return SUCCESS;
+}
+
 static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_undefined_method(const zend_class_entry *ce, const zend_string *method)
 {
        zend_throw_error(NULL, "Call to undefined method %s::%s()", ZSTR_VAL(ce->name), ZSTR_VAL(method));
@@ -2079,9 +2117,10 @@ num_undef:
                                retval = &EG(uninitialized_zval);
                                break;
                        case BP_VAR_RW:
-                               zend_undefined_offset(hval);
-                               retval = zend_hash_index_update(ht, hval, &EG(uninitialized_zval));
-                               break;
+                               if (UNEXPECTED(zend_undefined_offset_write(ht, hval) == FAILURE)) {
+                                       return NULL;
+                               }
+                               /* break missing intentionally */
                        case BP_VAR_W:
                                retval = zend_hash_index_add_new(ht, hval, &EG(uninitialized_zval));
                                break;
@@ -2109,7 +2148,9 @@ str_index:
                                                        retval = &EG(uninitialized_zval);
                                                        break;
                                                case BP_VAR_RW:
-                                                       zend_undefined_index(offset_key);
+                                                       if (UNEXPECTED(zend_undefined_index_write(ht, offset_key))) {
+                                                               return NULL;
+                                                       }
                                                        /* break missing intentionally */
                                                case BP_VAR_W:
                                                        ZVAL_NULL(retval);
@@ -2127,9 +2168,10 @@ str_index:
                                        retval = &EG(uninitialized_zval);
                                        break;
                                case BP_VAR_RW:
-                                       zend_undefined_index(offset_key);
-                                       retval = zend_hash_update(ht, offset_key, &EG(uninitialized_zval));
-                                       break;
+                                       if (UNEXPECTED(zend_undefined_index_write(ht, offset_key) == FAILURE)) {
+                                               return NULL;
+                                       }
+                                       /* break missing intentionally */
                                case BP_VAR_W:
                                        retval = zend_hash_add_new(ht, offset_key, &EG(uninitialized_zval));
                                        break;