php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #68118 $a->foo .= 'test'; can leave $a->foo undefined
Submitted: 2014-09-29 15:30 UTC Modified: 2014-10-03 20:05 UTC
From: rasmus@php.net Assigned: nikic
Status: Closed Package: Class/Object related
PHP Version: 5.5.17 OS: Any
Private report: No CVE-ID:
 [2014-09-29 15:30 UTC] rasmus@php.net
Description:
------------
PHP 5.5 introduced a new undefined property notice for this case:

$a->undefined .= 'test';

This means that if a custom error handler is in place it will be triggered, of course. There is a weird side-effect that can happen if the error handler hits the same error causing the property name to be overwritten. eg.

http://3v4l.org/KGXhF

Compare to:

http://3v4l.org/VPmLK

PHP 5.6 and PHP 7 are affected as well.

Test script:
---------------
<?php

class test {
    public function __construct() {
        $this->test = 'meow';
    }
}
 
function error_handler() {
    $test = new test();
    return true;
}
 
set_error_handler("error_handler");
 
// test one
class a {}
$a = new a;
$a->undefined .= 'test';
echo isset($a->undefined) ? 'true' : 'false';
 
echo "\n";
 
// test two
$b .= 'test';
echo isset($b) ? 'true' : 'false';
 
echo "\n";


Expected result:
----------------
true
true

Actual result:
--------------
false
true

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-09-30 14:10 UTC] nikic@php.net
-Status: Open +Status: Analyzed -Assigned To: +Assigned To: dmitry
 [2014-09-30 14:10 UTC] nikic@php.net
The problem is that zend_get_property_info_quick uses the global EG(std_property_info) in case of undefined properties (see http://lxr.php.net/xref/PHP_5_5/Zend/zend_object_handlers.c#343). If the "undefined property" notice is throw in zend_std_get_property_ptr_ptr (see http://lxr.php.net/xref/PHP_5_5/Zend/zend_object_handlers.c#757) and the error handler accesses an undefined property as well, the EG(std_property_info) value will be overwritten and the assignment will happen to the wrong property.

A simple solution would be to move the notice until after the property has been created. This will slightly change the behavior though, in that the error handler will be able to see the new property (whereas currently it can't).
 [2014-09-30 16:40 UTC] rasmus@php.net
I don't think letting the error handler see the new property is a problem. I can't think of an error handler flow where that is important. It seems like the cleanest way to fix this edge case.
 [2014-10-03 20:05 UTC] nikic@php.net
-Status: Analyzed +Status: Closed -Assigned To: dmitry +Assigned To: nikic
 [2014-10-03 20:05 UTC] nikic@php.net
I don't think it's important either, just wanted to mention it for the sake of completeness. I've moved the notice in https://github.com/php/php-src/commit/93288d0095cc27f8df88832e479e25ea5b00b5fd.
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Mon Apr 24 07:01:38 2017 UTC