Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:116865
To: Dan Ackroyd <Danack@basereality.com>
Cc: PHP internals <internals@lists.php.net>
References: <ded42b3e-3149-1192-0f86-07715fcb7a3e@woltlab.com>
 <CA+kxMuTVMeqVN1UFwau3eaVgLa2DKRKaErzSdFCKLw9v066mVQ@mail.gmail.com>
Message-ID: <01c3082c-c69d-7337-64f7-8314ade305a2@woltlab.com>
Date: Tue, 11 Jan 2022 10:11:20 +0100
MIME-Version: 1.0
In-Reply-To: <CA+kxMuTVMeqVN1UFwau3eaVgLa2DKRKaErzSdFCKLw9v066mVQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [PHP-DEV] RFC [Discussion]: Redacting parameters in back traces
From: duesterhus@woltlab.com (=?UTF-8?Q?Tim_D=c3=bcsterhus=2c_WoltLab_GmbH?=)

Hi Dan

On 1/10/22 6:01 PM, Dan Ackroyd wrote:
> How do other languages handle this problem? Or how do they avoid it in
> the first place?

Ryan already replied here, but I've also researched this:

- Java is unable to provide parameters in stack traces.
- In C you generally have a core dump which contains everything. 
Parameters might or might not be available depending on optimization level.
- In Python parameters are not provided by default, but it appears to be 
available via 
https://docs.python.org/3/library/inspect.html#inspect.getargvalues.
- In JavaScript the '.stack' property is non-standard and the behavior 
depends on the engine. Generally no parameters are provided.
- In Ruby parameters are not provided.

So basically all the other languages I researched do not provide 
arguments within back traces. I might have missed something, as I am not 
experienced in all of them.

>  From the RFC:
>> Specifically the back trace collection should be updated to use an object of class
>> \SensitiveParameter as the value for all parameters that are marked with the
>> \SensitiveParameter attribute.
> 
> To me....these words are not clear. Does the following sentence say
> the same thing?
> 
> "When the backtrace is generated, any parameter that has a
> 'SensitiveParameter' attribute will not have it's value stored in the
> backtrace, but instead will be replaced with an SensitiveParameter
> object.
> 
> If so, the RFC could be updated to be clearer.....if not, then the RFC
> should be updated to be clearer.

Yes, your phrasing is correct. It's much better than mine, so I've taken it.

> Also, having parameters replaced with another type doesn't seem
> obviously correct. There should probably be some words justifying why
> that is the correct thing to do, rather than just replacing any values
> with "****REDACTED***" or other simple behaviour.

I updated the RFC with a new "Choice of replacement value" section:

https://wiki.php.net/rfc/redact_parameters_in_back_traces#choice_of_replacement_value

>> On shared web hosting, the customer might not be able to configure it.
> 
> My personal opinion is that shared web hosting shouldn't be a thing
> that exists in 2022. And definitely shouldn't be used for anything
> where secrets need to be maintained. Yeah shared hosts might have a DB
> they can connect to, but those credentials should only be usuable from
> the shared host to the DB.

While I agree with regard to shared web hosting generally, I'd rather 
see a customer using a shared web hosting than attempting to self-host 
and running outdated vulnerable PHP versions.

Also while PDO is the prominent example at the beginning of the RFC, 
other functions are also affected. As an example 'ldap_bind()' might 
include a user's password when the directory is unavailable, possibly 
logging the user's password within Sentry or whatever error collector is 
in use.

Best regards
Tim Düsterhus
Developer WoltLab GmbH

-- 

WoltLab GmbH
Nedlitzer Str. 27B
14469 Potsdam

Tel.: +49 331 96784338

duesterhus@woltlab.com
www.woltlab.com

Managing director:
Marcel Werk

AG Potsdam HRB 26795 P