[zeriyoshi]

Found while reworking #22730.

FG(default_context) is freed with the request's resources, but the pointer is only reset in ext/standard's RINIT. The new stream error code (8.6+) falls back to it when no context is given, so an error raised before RINIT, for example a POST body over 16 KiB that cannot be spooled, reads the previous request's freed context: ASan reports a heap-use-after-free, a release build dies with SIGSEGV.

Clear FG(default_context) when its resource is destroyed. Only PHP-8.6 and master have the reader, 8.5 and earlier are not affected.

Test: sapi/cgi/tests/stream_error_default_context.phpt fails before the change and passes after it.

PoC:

printf 'display_errors=1\nlog_errors=0\nsys_temp_dir=/nonexistent\n' > php.ini
printf '<?php file_get_contents(__FILE__); echo "ok ", count($_POST), "\\n";' > ctx.php
head -c 20480 /dev/zero | tr '\0' a | sed 's/^/a=/' > body; cat body body body body body > body5
REDIRECT_STATUS=1 REQUEST_METHOD=POST SCRIPT_FILENAME=$PWD/ctx.php CONTENT_TYPE=application/x-www-form-urlencoded \
  CONTENT_LENGTH=20482 php-cgi -c php.ini -T 5 < body5; echo "exit $?"

gdb:

Program received signal SIGSEGV, Segmentation fault.
php_stream_error_operation_end (context=<optimized out>, context@entry=0x0) at main/streams/stream_errors.c:485
485			GC_ADDREF(context->res);
#1  php_stream_wrapper_error_internal (..., context=0x0, ...) at main/streams/stream_errors.c:624
#2  php_stream_wrapper_error (wrapper=0x0, context=0x0, ..., fmt="Unable to create temporary file, Check permissions in temporary files directory.") at main/streams/stream_errors.c:662
#3  php_stream_temp_write (stream=..., count=16384) at main/streams/memory.c:377
#7  sapi_read_standard_form_data () at main/SAPI.c:252
#9  sapi_activate () at main/SAPI.c:447
#10 php_request_startup () at main/main.c:1888

Debug + ASan build of the same head (USE_ZEND_ALLOC=0 USE_TRACKED_ALLOC=1):

ERROR: AddressSanitizer: heap-use-after-free on address 0x5030000148c8 ... READ of size 8
    #0 php_stream_context_get_option main/streams/streams.c:2377
    #1 php_stream_get_error_mode main/streams/stream_errors.c:122
    #2 php_stream_error_operation_end main/streams/stream_errors.c:479
    #3 php_stream_wrapper_error_internal main/streams/stream_errors.c:624
    #4 php_stream_wrapper_error main/streams/stream_errors.c:662
    #5 php_stream_temp_write main/streams/memory.c:377
    #6 php_stream_write_buffer main/streams/streams.c:1133
    #7 php_stream_write main/streams/streams.c:1278
    #8 sapi_read_standard_form_data main/SAPI.c:252
    #9 sapi_read_post_data main/SAPI.c:200
    #10 sapi_activate main/SAPI.c:447
    #11 php_request_startup main/main.c:1888
    #12 main sapi/cgi/cgi_main.c:2460
0x5030000148c8 is located 8 bytes inside of 32-byte region
freed by thread T0 here:
    #2 _efree Zend/zend_alloc.c:3052
    #3 php_stream_context_free main/streams/streams.c:2345
    #4 file_context_dtor ext/standard/file.c:118
    #5 zend_resource_dtor Zend/zend_list.c:72
    #6 zend_close_rsrc_list Zend/zend_list.c:227
    #7 zend_shutdown_executor_values Zend/zend_execute_API.c:284
    #8 shutdown_executor Zend/zend_execute_API.c:479
    #9 zend_deactivate Zend/zend.c:1367
    #10 php_request_shutdown main/main.c:2014
    #11 main sapi/cgi/cgi_main.c:2565
previously allocated by thread T0 here:
    #3 _ecalloc Zend/zend_alloc.c:3130
    #4 php_stream_context_alloc main/streams/streams.c:2352
    #5 zif_file_get_contents ext/standard/file.c:414
SUMMARY: AddressSanitizer: heap-use-after-free main/streams/streams.c:2377 in php_stream_context_get_option

[zeriyoshi]

@DanielEScherzer @mbeccati @svpernova09 Could this be pulled into PHP-8.6.0 for GA?

It fixes a use-after-free introduced in 8.6 by the stream error API (f2b371e). A POST body over 16 KiB that fails to spill to a temp file reports an error before RINIT, which falls back to FG(default_context) freed by the previous request. 8.5 is not affected, RC5 is. None of PHP-8.6, PHP-8.6.0 or master has an equivalent fix, and the patch applies cleanly to all three.

As 8.6 has no stable release yet, this is low severity under the security classification policy, so I filed it publicly. I'd like it to land before GA so it never ships in a stable release.

[zeriyoshi]

@mbeccati @bukka
The changes are minor, and if there are no issues, I will merge them into PHP-8.6.0 and PHP-8.6, master.
Is it necessary to include a note in the NEWS file for RC users?

[zeriyoshi]

(For PHP-8.6.0, we may need to re-tag some items. I'd like to leave this to RM.)

[mbeccati]

The changes are minor, and if there are no issues, I will merge them into PHP-8.6.0 and PHP-8.6, master.
Is it necessary to include a note in the NEWS file for RC users?

Yes please, go ahead with the merge and the appropriate NEWS entry, as soon as possible.

For PHP-8.6.0, we may need to re-tag some items

We will take care of cherry-picking into the PHP-8.6.0 branch, if that's what you mean.

[zeriyoshi]

@mbeccati
Following the precedent of GH-17951 in 8.5.0 (NEWS moved in 86b0a2a), I put the NEWS entry under 8.6.1 on PHP-8.6.

I've merged it up to master in 3cb920a, leaving the entry out of master's NEWS. As was done then, could you move it to the 8.6.0 section when cherry-picking it into PHP-8.6.0?