php / php-src

The PHP Interpreter
https://www.php.net
Other
37.57k stars 7.69k forks source link

Add Attribute::TARGET_PROPERTY to SensitiveParameter #14613

Closed delolmo closed 8 hours ago

delolmo commented 2 weeks ago

Description

Summary

Enhance the SensitiveParameter attribute to support object properties in addition to parameters. This extension aims to improve the security of sensitive data by ensuring that sensitive information within object properties is also redacted in stack traces and other relevant outputs.


The SensitiveParameter attribute in PHP is currently defined as follows:

This attribute is used to mark a parameter that is sensitive and should have its value redacted if present in a stack trace.

Extend the target of the SensitiveParameter attribute to include object properties. This would enable developers to mark object properties as sensitive, ensuring their values are redacted in stack traces and other logs where sensitive information should not be exposed.

To bear in mind:

The implementation would involve modifying the SensitiveParameter attribute class to support properties. This could be achieved by:

Example Usage:

class User
{
    #[SensitiveParameter]
    public string $password;

    #[SensitiveParameter]
    private string $apiKey;

    public function __construct(
        #[SensitiveParameter] string $password,
        #[SensitiveParameter] string $apiKey
    ) {
        $this->password = $password;
        $this->apiKey = $apiKey;
    }

    public function authenticate(): bool
    {
        // Authentication logic
    }
}

In this example, both the constructor parameters and the class properties password and apiKey are marked as sensitive. When an exception occurs, any stack trace or log containing these properties would redact their values, thus protecting sensitive information.

damianwadley commented 2 weeks ago

SensitiveParameter is specifically intended to cover the case of backtraces showing the values of function parameters. Redacting object properties is a completely separate use case. But also, applying a "sensitive parameter" attribute to a class property doesn't make sense.

Feature requests like this don't get a whole lot of visibility here in the bug tracker. I suggest you check out the RFC process, which starts with a conversation on the internals mailing list about what you have in mind.

iluuu1994 commented 2 weeks ago

Object properties are already not part of the stack trace:

https://3v4l.org/o0jpQ

Where/How exactly are you hitting this issue? It might be reasonable to add an attribute that automatically redacts the data for var_dump() and friends, i.e. something you'd need to use __debugInfo() for. But we obviously can't hide the property everywhere, like reflection. So, depending on your solution, this mitigation won't work.

Can you clarify what exactly you're doing and how you would expect the attribute to work?

github-actions[bot] commented 8 hours ago

No feedback was provided. The issue is being suspended because we assume that you are no longer experiencing the problem. If this is not the case and you are able to provide the information that was requested earlier, please do so. Thank you.