Skip to content

NULL-ptr dereference when using register_tick_function in destructor #18033

Closed

Description

@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

Activity

  1. ndossche commented on Mar 12, 2025

    @ndossche
    Member

    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).

  2. ndossche commented on Mar 12, 2025

    @ndossche
    Member

    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?

  3. bwoebi commented on Mar 12, 2025

    @bwoebi
    Member
    1. Make registering tick handlers in shutdown a no-op.

    But I think I prefer 1.

  4. arnaud-lb commented on Mar 12, 2025

    @arnaud-lb
    Member

    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.)

  5. iluuu1994 commented on Mar 13, 2025

    @iluuu1994
    Member

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

  6. added a commit that references this issue on Mar 13, 2025
    d0f7787
  7. added a commit that references this issue on Mar 14, 2025
    867ed15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions