Skip to content

Exception trace arguments can be modified by reference #18614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kkmuffme opened this issue May 21, 2025 · 4 comments
Closed

Exception trace arguments can be modified by reference #18614

kkmuffme opened this issue May 21, 2025 · 4 comments

Comments

@kkmuffme
Copy link

Description

The following code:

https://3v4l.org/1SGGd#v8.4.7

<?php

function byref( &$arg ) {
    $arg = 'abc';
}

function bar($a) {
    $x = new Exception();
    $trace = $x->getTrace();
    
    foreach ( $trace as $frame ) {
        if ( empty( $frame['args'] ) ) {
              continue;
        }
        
        foreach ( $frame['args'] as $key => $arg ) {
            if ( is_array( $arg ) ) {
                // array_walk_recursive( $arg, 'byref' );
                // same issue when doing it manually:
                foreach ( $arg as &$foo ) {
                    $foo = 'xyz';
                }
                unset( $foo );
            }
        }
    }
}

$b = array( 'X', 'Y' );
foreach ( $b as &$value ) {
    bar( $b );
    var_dump( $value );
}

Resulted in this output:

xyz
xyz

But I expected this output instead:

X
Y

The practical use case is that when an error (e.g. trigger_error) or exception is thrown and handled, that the error handler is able to modify params by reference (accidentally) that will possible later be used again.
This also means however, that by using a new Exception or possibly by causing an error, any code is suddenly able to modify variables by reference to which the code would usually not have access to.

Is this intended? If yes, do you think this is something that should be explicitly documented on the getTrace() doc page?

PHP Version

8.4.7 (error is there since forever though)

Operating System

No response

@bwoebi
Copy link
Member

bwoebi commented May 21, 2025

@kkmuffme The primary issue is that it is expensive. You'd have to iterate the nested arrays, searching for a reference, causing copies on the underlying arrays etc.

It's far more important to have fast collection than being fully proper here (if we decide this would not be desired). As such I'll have to close this as a wontfix at best and not a bug at worst.

Fwiw: Maybe worth a note, but a doc user comment there is also fine.

@bwoebi bwoebi closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2025
@kkmuffme
Copy link
Author

Is this a potential security issue though? You can essentially modify variable values (and again change them back later) that are outside of a function's scope without getting detected.

@bwoebi
Copy link
Member

bwoebi commented May 21, 2025

No, it is not. There's no such security guarantees within the PHP VM.

Also, the value has to be actually a reference.

@bwoebi
Copy link
Member

bwoebi commented May 21, 2025

Addendum: The main raison why we don't like spooky action at a distance of that type is because it impedes optimizations. But I believe - at least currently - we don't assume the type of references.

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

No branches or pull requests

2 participants