PHP 7.3 segfault running composer.phar

[rlerdorf]

php composer.phar segfaults under 7.3 with an invalid write in https://github.com/krakjoe/uopz/blob/master/src/handlers.c#513 Not much help from Valgrind on it:

==16026== Invalid write of size 8
==16026==    at 0x18168F8E: uopz_class_constant_handler (handlers.c:513)

but at least it is easy to reproduce.

[rlerdorf]

A short reproducing script:

<?php
class C {
    const F = 1;
}
echo C::F;

This doesn't segfault, but it triggers an uninitialized 8-byte read in the same place on the access to C::F there.

==22737== Use of uninitialised value of size 8
==22737==    at 0x18168F8E: uopz_class_constant_handler (handlers.c:513)
==22737==    by 0x8CBF68: ZEND_USER_OPCODE_SPEC_HANDLER (zend_vm_execute.h:1810)
==22737==    by 0x8CDE6E: execute_ex (zend_vm_execute.h:55510)
==22737==    by 0x8D6247: zend_execute (zend_vm_execute.h:60834)
==22737==    by 0x84E282: zend_execute_scripts (zend.c:1568)
==22737==    by 0x7EECAF: php_execute_script (main.c:2630)
==22737==    by 0x8D86C5: do_cli (php_cli.c:997)
==22737==    by 0x46504D: main (php_cli.c:1389)
==22737==  Uninitialised value was created by a heap allocation
==22737==    at 0x4C29B6D: malloc (vg_replace_malloc.c:298)
==22737==    by 0x4C2BB39: realloc (vg_replace_malloc.c:785)
==22737==    by 0x8252C8: __zend_realloc (zend_alloc.c:2923)
==22737==    by 0x82BD6D: zend_add_literal (zend_compile.c:501)
==22737==    by 0x83B3F8: zend_compile_class_decl (zend_compile.c:6403)
==22737==    by 0x83A6A7: zend_compile_stmt (zend_compile.c:8236)
==22737==    by 0x83D409: zend_compile_top_stmt (zend_compile.c:8142)
==22737==    by 0x83D44E: zend_compile_top_stmt (zend_compile.c:8137)
==22737==    by 0x81630C: zend_compile (zend_language_scanner.l:602)
==22737==    by 0x817715: compile_file (zend_language_scanner.l:636)
==22737==    by 0x17F3F73F: phar_compile_file (phar.c:3344)
==22737==    by 0x84E24B: zend_execute_scripts (zend.c:1562)
==22737==    by 0x7EECAF: php_execute_script (main.c:2630)
==22737==    by 0x8D86C5: do_cli (php_cli.c:997)
==22737==    by 0x46504D: main (php_cli.c:1389)

[rlerdorf]

Possible fix?

diff --git a/src/handlers.c b/src/handlers.c
index 8399f4a..68c804f 100644
--- a/src/handlers.c
+++ b/src/handlers.c
@@ -510,7 +510,11 @@ int uopz_class_constant_handler(UOPZ_OPCODE_HANDLER_ARGS) { /* {{{ */
                zend_string_release(key);
        }

+#if PHP_VERSION_ID < 70300
        CACHE_PTR(Z_CACHE_SLOT_P(EX_CONSTANT(EX(opline)->op2)), NULL);
+#else
+       CACHE_PTR(EX(opline)->extended_value, NULL);
+#endif

        if (uopz_fetch_class_constant_handler) {
                return uopz_fetch_class_constant_handler(UOPZ_OPCODE_HANDLER_ARGS_PASSTHRU);

This stops the segfault on composer.phar and valgrind is clean

[rlerdorf]

Ah, but there is a failing test now with this patch: FAIL uopz_set_mock [tests/004.phpt]

[rlerdorf]

@nikic or @dstogov could you help out @krakjoe and I in this src/handler.c file? There are a number of blocks similar to:

#if PHP_VERSION_ID >= 70300
                    /* I just, I dunno */
#else
                    CACHE_PTR(Z_CACHE_SLOT_P(function_name), NULL);
#endif

in addition to the one I was trying to fix here.

[krakjoe]

I'm not able to reproduce the original memory error, so I'm kinda stabbing in the dark ... but I think the commit above looks correct to me ...

I'm also looking at the "I dunno" comments ... any help from anywhere is very appreciated ...

[krakjoe]

c62b06d204373e9dd7c99d106664462b36d4acaa attempts to fix the "I dunno" bits ...

[dstogov]

@rlerdorf @krakjoe I'll try to find time on next week. "I dunno" is definitely should be fixed accordingly to 7.3 changes.

It might make sense to include this " very core dependent" extension into PHP distribution, to implement its support together with core changes...

[dstogov]

I've fixed incorrect run-time cache usage.

[rlerdorf]

I've fixed incorrect run-time cache usage.

Awesome, thanks Dmitry!

[kornrunner]

I think something is still off (possibly included by some other PR/merge).

So far, I've tested on 7.2.13, current uopz master compiled - make test passes, but composer still segfaults (haven't tried 7.3 yet).

I suspect other (recent) changes are causing the segfault, as everything worked just fine with 5.0.2 tag.

I'll see to compile both 7.2 and 7.3 with debug and provide some additional info.

[rlerdorf]

It isn't segfaulting running composer.phar under 7.3 for me now

[kornrunner]

I can confirm as well that composer doesn't segfault under 7.3.

[krakjoe]

So I'm closing this then ... we're probably going to have a bit more work before a release is ready ... thanks for input everyone :)