Repository navigation
System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations #17733
Description
Description
The following code:
<?php
final class SodiumBase64Polyfill
{
public const SODIUM_BASE64_VARIANT_ORIGINAL_NO_PADDING = 3;
public const SODIUM_BASE64_VARIANT_URLSAFE = 5;
public const SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING = 7;
public static function bin2base64(string $decoded, int $variant): string
{
if (! function_exists('sodium_bin2base64')) {
return self::bin2base64Fallback($decoded, $variant);
}
return sodium_bin2base64($decoded, $variant);
}
public static function bin2base64Fallback(string $decoded, int $variant): string
{
$encoded = base64_encode($decoded);
if (
$variant === self::SODIUM_BASE64_VARIANT_URLSAFE
|| $variant === self::SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING
) {
$encoded = strtr($encoded, '+/', '-_');
}
if (
$variant === self::SODIUM_BASE64_VARIANT_ORIGINAL_NO_PADDING
|| $variant === self::SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING
) {
$encoded = rtrim($encoded, '=');
}
return $encoded;
}
}
var_dump(SodiumBase64Polyfill::bin2base64("Hello world!", SodiumBase64Polyfill::SODIUM_BASE64_VARIANT_URLSAFE_NO_PADDING));Resulted in this output:
"Hello world!�"
But I expected this output instead:
"SGVsbG8gd29ybGQh"
Steps to reproduce
- Run PHP-FPM with the following php.ini settings:
zend_extension=opcache.so
extension=sodium.so
opcache.enable = 1
opcache.enable_cli = 1
; Tune opcache for read-only application files
opcache.revalidate_freq=0
opcache.validate_timestamps=false
opcache.enable_file_override=1
; Enable pre-warmed opcache
opcache.file_cache = "/var/cache/opcache"
opcache.file_cache_read_only=1
opcache.file_cache_consistency_checks=0
; Enable log
opcache.error_log=/dev/stderr
opcache.log_verbosity_level = 4- Compile and run the script using:
php -d zend_extension=opcache.so \
-d extension=sodium.so \
-d opcache.enable=1 \
-d opcache.enable_cli=1 \
-d opcache.jit=disable \
-d opcache.file_cache=/var/cache/opcache \
-d opcache.file_cache_only=1 \
-d opcache.file_cache_read_only=0 ./public/index.php- Send request php-fpm
Additional information
- The issue disappears when opcache.optimization_level=0 is set.
- The bug is tested with the file_cache_only feature introduced in PR #16551, but the issue also reproduces in PHP 8.4.
PHP Version
master, 8.4
Operating System
linux x86 or aarch64
Activity
ndossche commentedon Feb 7, 2025
This peaks my interest. I'll have a look now and try to reproduce.
bashkarev commentedon Feb 7, 2025
@nielsdos
I edited the php.ini block during debugging and forgot to remove the opcache.optimization_level=0 setting. If you need any further assistance in reproducing the issue, please let me know.
ndossche commentedon Feb 7, 2025
I can reproduce this, you don't even need php-fpm for this.
First, run the script to prime the file cache, with sodium extension loaded.
Then, trigger the crash by running the script with sodium extension not loaded.
I suppose php-fpm does not have sodium loaded in your configuration.
bashkarev commentedon Feb 7, 2025
Yes, it is generated at the Docker build stage. I will now try to create a Compose setup for reproduction.
ndossche commentedon Feb 7, 2025
So the particular crash you see is caused here, by the offset value that will be invalid:
php-src/Zend/zend_vm_def.h
Lines 4046 to 4049 in d3e5dbe
This was introduced in 980f7bd
While this is not the root cause, there may be an additional problem here:
If the load order of the extensions change between priming the file cache and using the file cache then the offset is also wrong
In OP's case, the bug was not introduced by that commit, but existed earlier because if we undo that commit we now get:
This makes sense, the optimizer uses information about extensions to optimize the calls, but if we rerun without a particular extension loaded then that information is invalid.
On first sight, we may need to add some checks or need to fingerprint the system id with info about the extensions. Probably the latter. Something worth discussing in group I think.
cc @iluuu1994 what do you think
bashkarev commentedon Feb 7, 2025
Sodium is included as a shared library in both cases.
docker-compose https://github.com/bashkarev/issues-17733
ndossche commentedon Feb 7, 2025
Thanks for the reproducing repo. In that case it's then likely caused by the ordering of extensions (or more precise: the ordering of internal functions) problem that I described in #17733 (comment)
iluuu1994 commentedon Feb 7, 2025
Indeed, the file cache serializer should drop the function table offset for
ZEND_INIT_FCALLthat may diverge in subsequent requests. And as @nielsdos has mentioned, we would also need to de-optimize toZEND_INIT_FCALL_BY_NAME, which is very unfortunate. Alternatively, we could try to discard the cache if the ini-configuration has changed in a way that could influence compilation results (loaded extensions, opcache settings, etc.). That seems like the better approach, but isn't trivial.ndossche commentedon Feb 7, 2025
I don't think the opcache ini settings matter for the interference against the optimizer between requests. Or at least I can't think of anything right off the bat.
I would think that looping over the extensions in-order and mixing their names (in-order) to the hash in
zend_finalize_system_idshould suffice for this particular issue, but not sure if this is complete, but it should cover the most important problem.iluuu1994 commentedon Feb 7, 2025
@nielsdos There are at least some other INI settings, like
disable_functions, where the function is no longer available in the next request. We should fly over the settings at least to see if any of them are problematic.13 remaining items
ndossche commentedon Nov 29, 2025
But this isn't actually fixed... it's workarounded but can still be hit in other ways
splitice commentedon Nov 29, 2025
The workaround appears to work for me. I'm not able to do a full validation run yet as we are dependent on xdebug (which does not have a 8.5 release yet)
bukka commentedon Nov 29, 2025
ah ok.
iluuu1994 commentedon Nov 29, 2025
@splitice Note that the fix also landed in 8.4.15.
splitice commentedon Nov 30, 2025
I got the project verified to pass all unit tests against xdebug alpha. I'd estimate the workaround has restored things back to working as per PHP 8.3.
Look forward to seeing a more complete fix.
[-]Incorrect base64 encoding in PHP-FPM with opcache.file_cache_only=1 and sodium[/-][+]System entropy does not take into account function order, resulting in wrong functions possibly called in different configurations[/+]Fix phpGH-17733: System entropy does not take into account function o…
Merge branch 'PHP-8.4' into PHP-8.5
Merge branch 'PHP-8.5' into PHP-8.6
Merge branch 'PHP-8.6'