Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak when calling a static method inside an xpath query #11347

Closed
flmommens opened this issue May 30, 2023 · 1 comment · Fixed by ThePHPF/thephp.foundation#90
Closed

Comments

@flmommens
Copy link

Description

Calling a static method inside an xpath query like so
$xpath->query("//a[php:function('MyClass::myMethod()')]")
generates a memory leak with PHP 8.1 and PHP 8.2. It used to work fine with PHP 7.2.

I have written a minimal script to reproduce the issue.

<?php

class MyClass
{
    public static function doesNothing(string $var): bool {
        return true;
    }
}

libxml_use_internal_errors(true);
$html = file_get_contents('hacks.html');
$doc = new DOMDocument();
$i = 0;
while(true) {
    $doc->loadHTML($html);
    $xpath = new DOMXpath($doc);
    $xpath->registerNamespace("php", "http://php.net/xpath");
    $xpath->registerPHPFunctions();
    $list =  $xpath->query("//a[php:function('MyClass::doesNothing', string(@href))]");
    echo $i++, "\r";
    libxml_clear_errors();
    usleep(10000);
}

I'm running PHP 8.1.18 and PHP 8.2.6 from Ondřej Surý packages on a Ubuntu 22.04 server. The problem is also present with the vanilla PHP (8.1.2).

I'm attaching the zipped hacks.html used in the above code for convenience but any HTML will trigger the issue.
hacks.zip

PHP Version

PHP 8.2.6

Operating System

Ubuntu 22.04

@nielsdos
Copy link
Member

It's a type confusion bug, kinda. zend_make_callable may change the function name of the fci to become an array, causing a crash in debug mode on zval_ptr_dtor_str(&fci.function_name); in dom_xpath_ext_function_php. On a production build it doesn't crash but only causes a leak.
I'll also check the other zend_make_callable cases.

nielsdos added a commit to nielsdos/php-src that referenced this issue May 30, 2023
…path query

It's a type confusion bug. `zend_make_callable` may change the function name
of the fci to become an array, causing a crash in debug mode on
`zval_ptr_dtor_str(&fci.function_name);` in `dom_xpath_ext_function_php`.
On a production build it doesn't crash but only causes a leak, because
the array elements are not destroyed, only the array container itself
is.
nielsdos added a commit to nielsdos/php-src that referenced this issue May 30, 2023
…path query

It's a type confusion bug. `zend_make_callable` may change the function name
of the fci to become an array, causing a crash in debug mode on
`zval_ptr_dtor_str(&fci.function_name);` in `dom_xpath_ext_function_php`.
On a production build it doesn't crash but only causes a leak, because
the array elements are not destroyed, only the array container itself
is. We can use the nogc variant because it cannot contain cycles, the
potential array can only contain 2 strings.
nielsdos added a commit that referenced this issue May 31, 2023
* PHP-8.1:
  Fix GH-11347: Memory leak when calling a static method inside an xpath query
nielsdos added a commit that referenced this issue May 31, 2023
* PHP-8.2:
  Fix GH-11347: Memory leak when calling a static method inside an xpath query
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants