NULL-ptr dereference when using `register_tick_function` in destructor

[clesmian]

Description

The following code:

<?php
class foo{
  function __destruct(){
    declare(ticks=1);
    register_tick_function(
       function() { }
    );
  }
}

$bar=new foo;
?>

Results in a segmentation fault of the OSS-Fuzz harness php-fuzz-function-jit & php-fuzz-tracing-jit. It appears that the code snippet executes fine without jit, but crashes with the following stacktrace when jit is enabled:

    #0 0x55bf976bdfc4 in zend_llist_apply /src/php-src/Zend/zend_llist.c:183
    #1 0x55bf976be434 in zend_llist_apply_with_argument /src/php-src/Zend/zend_llist.c:236:3
    #2 0x55bf97107be3 in php_run_ticks /src/php-src/main/php_ticks.c:68:2
    #3 0x55bf974e3214 in ZEND_TICKS_SPEC_HANDLER /src/php-src/Zend/zend_vm_execute.h:3154:4
    #4 0x55bf973e716b in execute_ex /src/php-src/Zend/zend_vm_execute.h:58595:7
    #5 0x55bf973e7ba2 in zend_execute /src/php-src/Zend/zend_vm_execute.h:64247:2
    #6 0x55bf9774e0f3 in fuzzer_do_request_from_buffer /src/php-src/sapi/fuzzer/fuzzer-sapi.c:274:5
    #7 0x55bf9774c92e in LLVMFuzzerTestOneInput /src/php-src/sapi/fuzzer/fuzzer-tracing-jit.c:43:3
    #8 0x55bf96a1f020 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
    #9 0x55bf96a0a295 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
    #10 0x55bf96a0fd2f in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
    #11 0x55bf96a3afd2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #12 0x7f271614b082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 0702430aef5fa3dda43986563e9ffcc47efbd75e)
    #13 0x55bf96a0247d in _start (/out/php-fuzz-tracing-jit+0x80247d)

We discovered this crash by fuzzing using the OSS-Fuzz infrastructure. We verified the issue to exist in commit 8731c95

PHP Version

master

Operating System

No response

[nielsdos]

This isn't JIT specific or anything. You can reproduce it outside of JIT by using the --repeat CLI option too.
The problem is that php_request_shutdown calls php_deactivate_ticks prior to running destructors and the shutdown functions. So if a destructor or shutdown function re-registers a tick function, then the user tick functions handler will be added back to PG(tick_functions). When the next request happens, the list PG(tick_functions) still contains an entry to call the user tick functions (added in the previous request during shutdown as I said). This causes a NULL deref eventually because run_user_tick_functions assumes that if it is called then BG(user_tick_functions) must be non-NULL (which is a sensible invariant).

[nielsdos]

Some solution ideas:

1. We could move php_deactivate_ticks below zend_call_destructors in php_request_shutdown. But this is a behaviour change observable to userspace. Arguably the cleanest imo
2. We could keep the current php_deactivate_ticks and add another call below zend_call_destructors, this is no behaviour change but is ugly.
3. We could reset the tick function in the request startup, but this is also ugly and I don't like that it crosses request boundary in the first place.

cc @iluuu1994 @arnaud-lb Suggestions on what you prefer?

[bwoebi]

4. Make registering tick handlers in shutdown a no-op.

But I think I prefer 1.

[arnaud-lb]

I would consider the current behaviour a bug, as destructors should behave similarly when executed before or during shutdown. For this reason I prefer solution 1, at least in master.

Error handlers can also be executed during shutdown, so there shouldn't be (more) issues. (Only issue I can think of is that destructors of the tick function dependencies may have already been called, but this can also happen before shutdown.)

[iluuu1994]

Solution 1. also sounds best to me. I'd prefer targetting master, given nobody encountered this issue in a production system.