[pull] master from php:master#1053
Merged
Merged
Conversation
…reset
The code tries to read the context on NULL when
`php_stream_xport_crypto_setup` fails because by then `stream` is reset
to NULL.
This is also UB, so can cause miscompiles.
```
==1217==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000090 (pc 0x55d829ed3acf bp 0x7fff045f5770 sp 0x7fff045f4df0 T0)
==1217==The signal is caused by a READ memory access.
==1217==Hint: address points to the zero page.
#0 0x55d829ed3acf in php_stream_url_wrap_http_ex /work/php-src/ext/standard/http_fopen_wrapper.c:580
#1 0x55d829ed857e in php_stream_url_wrap_http /work/php-src/ext/standard/http_fopen_wrapper.c:1204
#2 0x55d82a15073d in _php_stream_open_wrapper_ex /work/php-src/main/streams/streams.c:2270
#3 0x55d829e78fa6 in zif_file_get_contents /work/php-src/ext/standard/file.c:409
#4 0x55d829bbfe39 in zif_phar_file_get_contents /work/php-src/ext/phar/func_interceptors.c:226
#5 0x55d82a0b7ed2 in zend_test_execute_internal /work/php-src/ext/zend_test/observer.c:306
#6 0x55d82a3e024a in ZEND_DO_FCALL_SPEC_RETVAL_USED_HANDLER /work/php-src/Zend/zend_vm_execute.h:2154
#7 0x55d82a540995 in execute_ex /work/php-src/Zend/zend_vm_execute.h:116519
#8 0x55d82a5558b0 in zend_execute /work/php-src/Zend/zend_vm_execute.h:121962
#9 0x55d82a6ba0ab in zend_execute_script /work/php-src/Zend/zend.c:1980
#10 0x55d82a0ec8bb in php_execute_script_ex /work/php-src/main/main.c:2645
#11 0x55d82a0ecccb in php_execute_script /work/php-src/main/main.c:2685
#12 0x55d82a6bfc16 in do_cli /work/php-src/sapi/cli/php_cli.c:951
#13 0x55d82a6c21e3 in main /work/php-src/sapi/cli/php_cli.c:1362
#14 0x7f9e770491c9 (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#15 0x7f9e7704928a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
#16 0x55d829209b34 in _start (/work/php-src/build-dbg-asan/sapi/cli/php+0x609b34) (BuildId: aa149f943514fff0c491e1f199e30fed0e977f7c)
```
This is a backport of:
* 7782b88
* f9519cc
* afded3d
Co-authored-by: Ilija Tovilo <ilija.tovilo@me.com>
* PHP-8.3: PHP-8.3 is now for PHP 8.3.33-dev Fix NULL deref when enabling TLS fails and the peer name needs to be reset
* PHP-8.4: PHP-8.3 is now for PHP 8.3.33-dev Fix NULL deref when enabling TLS fails and the peer name needs to be reset
* PHP-8.5: PHP-8.3 is now for PHP 8.3.33-dev Fix NULL deref when enabling TLS fails and the peer name needs to be reset
…L stream wrapper This issue is related to GH-22332: > Let's assume we have a stream with 5 queued bytes. When calling fread($s, 1), > the underlying buffered stream will request a chunk of up to 8192 bytes > immediately, but only return the requested 1 byte back to the reader. If the > reader then requests fread($s, 10), _php_stream_read() will first return > anything that was buffered but not yet read. > > If the requested length exceeds the number of buffered bytes (as is the case > above), another read call is issued. This call will return nothing, because > the stream only provides 4 more readable bytes, all of which are buffered. > php_openssl_handle_ssl_error() (called by php_openssl_sockop_io()) will then > incorrectly set last_status to WANT_READ, even though we've already read the > remaining data. > > Furthermore, stream_select() can cause the same issue via > php_openssl_sockop_cast(castas: PHP_STREAM_AS_FD_FOR_SELECT), which pre-fills > the read buffer on SSL_pending() > 0. The subsequent fread() will lead to the > same condition as above. > > There's a second issue here. If the stream is blocking, the supplement read > will block for the duration of the timeout. This will be addressed in a second > PR. This addresses the last paragraph. Avoid a blocking read when we already have buffered data, as we may have reached the end of the stream and will wait in vain. Closes GH-22346
* PHP-8.4: Fixed timeout for supplemental read at end of a blocking stream in SSL stream wrapper
* PHP-8.5: Fixed timeout for supplemental read at end of a blocking stream in SSL stream wrapper
Let's assume we have a stream with 5 queued bytes. When calling fread($s, 1), the underlying buffered stream will request a chunk of up to 8192 bytes immediately, but only return the requested 1 byte back to the reader. If the reader then requests fread($s, 10), _php_stream_read() will first return anything that was buffered but not yet read. If the requested length exceeds the number of buffered bytes (as is the case above), another read call is issued. This call will return nothing, because the stream only provides 4 more readable bytes, all of which are buffered. php_openssl_handle_ssl_error() (called by php_openssl_sockop_io()) will then incorrectly set last_status to WANT_READ, even though we've already read the remaining data. Furthermore, stream_select() can cause the same issue via php_openssl_sockop_cast(castas: PHP_STREAM_AS_FD_FOR_SELECT), which pre-fills the read buffer on SSL_pending() > 0. The subsequent fread() will lead to the same condition as above. Closes GH-22332
…rait preload_fix_trait_op_array rewrites the clone op_array from its original after optimization, preserving function_name, scope, fn_flags, prototype, and static_variables. For trait-cloned property hooks, it also needs to preserve prop_info: zend_do_traits_property_binding set it to the using class's property, but the reset pointed it back at the trait's property. That mismatch caused zend_is_in_hook to miss the self-access check inside the hook, recursing into the getter/setter instead of reading or writing the backing store. Fixes GH-21770 Closes GH-21788
* PHP-8.4: Fix infinite recursion in property hook getter in opcache preloaded trait
* PHP-8.5: Fix infinite recursion in property hook getter in opcache preloaded trait
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )