Skip to content

Conversation

@arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Oct 21, 2025

Typed references may be modified while assigning to them, during coercion (#20318):

  • The reference may be freed, resulting in UAF
  • The type source list maybe freed or reallocated, resulting in UAF
  • Some newly added types may skipped, resulting in incorrect typing

The rvalue may also be changed, leading to UAFs or leaks when freeing it (#20316).

Here we fix these issues (see commits).

Freeing is avoided by increasing the refcount during assignment.

Source list issues are fixed by updating the iteration code, and modifying the list in append-only mode during assignment.

zend_verify_ref_assignable_zval() repeats coercion for every type source, to ensure consistency. Since each execution of a __toString() may append new types, this could easily turn into an infinite loop. To avoid this, we execute __toString() only once, assuming that converting the same object to string twice will have the same result.

The rvalue issue is fixed by copying the zval to a stack slot.

Fixes GH-20318, GH-20316.

Targeting master only, as the type list issue is difficult to fix in release branches.

@github-actions
Copy link

AWS x86_64 (c7i.24xl)

Attribute Value
Environment aws
Runner host
Instance type c7i.metal-24xl (dedicated)
Architecture x86_64
CPU 48 cores
CPU settings disabled deeper C-states, disabled turbo boost, disabled hyper-threading
RAM 188 GB
Kernel 6.1.147-172.266.amzn2023.x86_64
OS Amazon Linux 2023.8.20250818
GCC 14.2.1
Time 2025-10-21 13:53:38 UTC

Laravel 12.2.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)

PHP Min Max Std dev Rel std dev % Mean Mean diff % Median Median diff % Skew P-value Instr count Memory
PHP - baseline@02d1 0.46731 0.46966 0.00046 0.10% 0.46837 0.00% 0.46834 0.00% 0.423 0.999 176200303 44.25 MB
PHP - gh15938-refs 0.46598 0.46910 0.00047 0.10% 0.46698 -0.30% 0.46693 -0.30% 0.971 0.000 176202220 44.25 MB

Symfony 2.7.0 demo app - 100 consecutive runs, 50 warmups, 100 requests (sec)

PHP Min Max Std dev Rel std dev % Mean Mean diff % Median Median diff % Skew P-value Instr count Memory
PHP - baseline@02d1 0.73109 0.73682 0.00114 0.16% 0.73287 0.00% 0.73262 0.00% 1.000 0.999 287286188 40.48 MB
PHP - gh15938-refs 0.73201 0.73965 0.00152 0.21% 0.73411 0.17% 0.73382 0.16% 1.910 0.000 287286009 40.51 MB

Wordpress 6.2 main page - 100 consecutive runs, 20 warmups, 20 requests (sec)

PHP Min Max Std dev Rel std dev % Mean Mean diff % Median Median diff % Skew P-value Instr count Memory
PHP - baseline@02d1 0.58089 0.58618 0.00102 0.17% 0.58310 0.00% 0.58315 0.00% 0.082 0.999 1120049118 43.97 MB
PHP - gh15938-refs 0.57664 0.58460 0.00103 0.18% 0.58218 -0.16% 0.58233 -0.14% -1.732 0.000 1120046513 43.97 MB

bench.php - 100 consecutive runs, 10 warmups, 2 requests (sec)

PHP Min Max Std dev Rel std dev % Mean Mean diff % Median Median diff % Skew P-value Instr count Memory
PHP - baseline@02d1 0.42370 0.43657 0.00266 0.62% 0.42804 0.00% 0.42732 0.00% 1.407 0.999 2020586579 26.90 MB
PHP - gh15938-refs 0.42280 0.44114 0.00304 0.71% 0.43049 0.57% 0.42960 0.53% 1.712 0.000 2020566529 26.90 MB

@arnaud-lb arnaud-lb force-pushed the gh15938-refs branch 2 times, most recently from 73824f2 to df25942 Compare October 28, 2025 15:49
Typed references may be modified while assigning to them, during coercion:
 * The reference may be freed, resulting in UAF
 * The type source list maybe freed or reallocated, resulting in UAF
 * Some newly added types may skipped, resulting in incorrect typing

Here we fix these issues.

Freeing is avoided by increasing the refcount during assignment.

Source list issues are fixed by updating the iteration code, and modifying the
list in append-only mode during assignment.
…e=IS_TMP_VAR

zend_assign_to_typed_ref_ex() receives a zval slot and tries to dtor it when
value_type=IS_TMP_VAR. This slot may be modified before dtor if it was in fact a
CV slot. In particular it may be turned into a ref, replaced by anyother ref, or
unset; leading to UAFs or leaks.

Fix by passing a stack slot instead. This is consistent with
zend_assign_to_typed_prop().
@arnaud-lb arnaud-lb marked this pull request as ready for review October 28, 2025 16:25
@arnaud-lb arnaud-lb requested a review from dstogov as a code owner October 28, 2025 16:25
@arnaud-lb arnaud-lb changed the title Handle concurrent mutations during reference assignment Fix GH-20318, GH-20316: Handle concurrent mutations during reference assignment Oct 30, 2025
Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These attempts to fix "destructive" magic methods became too complicated for me.
Take the decision yourself. I won't object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Side effects during zend_assign_to_typed_ref_ex() may modify the reference being assigned to

2 participants