php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #71266 Missing separation of properties HT in foreach etc.
Submitted: 2016-01-03 12:08 UTC Modified: 2016-01-18 10:20 UTC
From: nikic@php.net Assigned: dmitry (profile)
Status: Closed Package: Scripting Engine problem
PHP Version: 7.0Git-2016-01-03 (Git) OS:
Private report: No CVE-ID: None
View Add Comment Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
You can add a comment by following this link or if you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: nikic@php.net
New email:
PHP Version: OS:

 

 [2016-01-03 12:08 UTC] nikic@php.net
Description:
------------
There are currently a number of places in the engine and library where the properties table is not properly separated. Essentially it's all the cases where get_properties() (or OBJPROP or HASH_OF) is used in a write context.

Two examples (assuming no immutable arrays):

$obj = (object) ['foo' => 1, 'bar' => 2];
foreach ($obj as $val) {
    var_dump($val);
    $obj->bar = 42;
}

This outputs 1, 1, 42 instead of 1, 42, because the properties table will be separated at the assignment, while it should have been separated at the start of the loop.

$arr = ['foo' => 1, 'bar' => 2];
$obj = (object) $arr;
next($obj);
var_dump(current($arr));

This outputs 2 instead of 1, because next() did not separate the properties HT, so it is still shared with the array.

I'm not sure how we should best fix this. We could always separate in get_properties(), however not all get_properties() uses are in write context. We can't cleanly separate outside get_properties(), as we'd have to assume how the properties are stored. The best option would likely be an extra argument to determine read/write context, but that's not ABI compatible.


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-01-03 13:36 UTC] laruence@php.net
For case 1:

   maybe we should not reset iter->pos if ht is updated(ht != iter->ht).
  
   something like:
diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c
index 11034fe..c44c689 100644
--- a/Zend/zend_hash.c
+++ b/Zend/zend_hash.c
@@ -381,7 +381,6 @@ ZEND_API HashPosition ZEND_FASTCALL zend_hash_iterator_pos(uint32_t idx, HashTab
                        ht->u.v.nIteratorsCount++;
                }
                iter->ht = ht;
-           iter->pos = ht->nInternalPointer;
        }
        return iter->pos;
 }
@@ -405,7 +404,6 @@ ZEND_API HashPosition ZEND_FASTCALL zend_hash_iterator_pos_ex(uint32_t idx, zval
                        ht->u.v.nIteratorsCount++;
                }
                iter->ht = ht;
-           iter->pos = ht->nInternalPointer;
        }
        return iter->pos;
 }

for case 2, maybe we could simple separate the array?

thanks
 [2016-01-18 10:20 UTC] dmitry@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: dmitry
 [2016-06-07 20:19 UTC] dmitry@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=421843977f3f2f65c2e7d6d2827b322a54042422
Log: Fixed bug #71266 (Missing separation of properties HT in foreach etc).
 [2016-06-07 20:19 UTC] dmitry@php.net
-Status: Assigned +Status: Closed
 [2016-06-22 05:58 UTC] krakjoe@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=421843977f3f2f65c2e7d6d2827b322a54042422
Log: Fixed bug #71266 (Missing separation of properties HT in foreach etc).
 [2016-07-20 11:30 UTC] davey@php.net
Automatic comment on behalf of dmitry@zend.com
Revision: http://git.php.net/?p=php-src.git;a=commit;h=421843977f3f2f65c2e7d6d2827b322a54042422
Log: Fixed bug #71266 (Missing separation of properties HT in foreach etc).
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Mar 28 08:01:28 2024 UTC