#[\SensitiveParameter] should be inherited if unmodified param is used?

[kkmuffme]

Description

https://3v4l.org/VQhiC#v8.4.6

<?php

function some_lib( $auth ) {
    throw new Exception( 'Some error' );
}

function my_code(
    #[\SensitiveParameter] 
    $password
) {
    some_lib( $password );
}

try {
    my_code('hunter2');
} catch ( Throwable $e ) {
    var_dump( $e->getTrace() );
}

At the moment, SensitiveParameter only makes sense if the function does not pass this parameter to any other function (unless that other function also has SensitiveParameter implemented), as that other function will leak the sensitive parameter anyway.
This creates a false sense of security.

Possible options:

1. if the param is passed to another function call, ignore the sensitive parameter alltogether, since it's pointless just marking it sensitive in 1 trace frame
   or
2. if the param is used without modification, it should pass on that "sensitive" even to a function, where the param is not marked sensitive.

[bwoebi]

I think this is pointless. You could add an exception for unmodified passing, theoretically.
And then you want to extend that to cases where the parameter is embedded inside a string etc.

PHP does not really have any sort of tainting (which this effectively is).

What you actually want, is changing the some_lib function (this might mean an upstream PR, then, yes.)

[staabm]

this sounds like something which could easily be errored about via static analysis.

maybe open a feature request at e.g. PHPStan to make it error when parameters attributed #[\SensitiveParameter] are passed to another Call-Like which does not mark it as #[\SensitiveParameter]

[iluuu1994]

Agreed. Alternatively, you can pass around a wrapper that hides your value. https://3v4l.org/1ujDU

if the param is passed to another function call, ignore the sensitive parameter alltogether

That seems much worse, no?

[TimWolla]

Alternatively, you can pass around a wrapper that hides your value. https://3v4l.org/1ujDU

You mean the native SensitiveParameterValue class? 😄

---

As the RFC author and from my experience of using the attribute in practice: It is rare to pass a sensitive value to some “generic function” that does not (and cannot) have the attribute and that also is capable of throwing an exception. Usually you are just passing along a $password parameter to another $password parameter, and clearly both of the $password parameters should have the attribute. You are not usually passing a password to str_replace() or similar (and even then, str_replace() is not going to throw).

[kkmuffme]

@staabm

this sounds like something which could easily be errored about via static analysis.
maybe open a feature request at e.g. PHPStan to make it error when parameters attributed #[\SensitiveParameter] are passed to another Call-Like which does not mark it as #[\SensitiveParameter]

The issue isn't in my own code - the problem is in libraries and/or PHP extensions, which I can't change.

@iluuu1994

That seems much worse, no?

No, it's the same. Whether I leak a user's password in the nth or the xth frame of a stack trace doesn't matter. A leak is a leak.

@TimWolla

You are not usually passing a password to str_replace() or similar (and even then, str_replace() is not going to throw).

In practice, the more common issue is not that a function that explicitly uses the parameter throws, but something unrelated, but the $password is there as param in the stack trace. e.g.

function foo( $host, $pw ) {
    $c = connect_something( $host ); // this throws if it fails, $pw isn't used in this call at all, but it will be in the trace
    auth_something( $c, $pw );
    return $c;
}

Currently (and since long before SensitiveParameter was introduced), I'm just scrubbing all sensitive data based on the param name from trace frames in the exception handler. Which made me wonder what the purpose of SensitiveParameter is, when I have to manually scrub it anyway.
(this also means that this suggested taint behavior is something I implemented in userland now. I just thought I'll open it as an issue, since SensitiveParameter is a great thing especially for simple projects, that do not do any custom exception handling. Since it could ensure no sensitive data is leaked to logs,... However to truly achieve this, additional userland handling is currently needed)

[iluuu1994]

What do you suggest then? Something that is ever passed as a sensitive parameter is forever tainted and will never be shown in stack traces? How do you suggest implementing this, especially without a performance impact?

The issue isn't in my own code - the problem is in libraries and/or PHP extensions, which I can't change.

Maybe that is the root of your issue then.