Opened 12 months ago
Last modified 9 months ago
#57304 new enhancement
Add SensitiveParameter attribute to DB connection and login variables
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | |
Component: | Security | Keywords: | php82 has-patch |
Focuses: | Cc: |
Description
PHP 8.2 introduces a SensitiveParameter attribute that can "mark a parameter that is sensitive and should have its value redacted if present in a stack trace."
WordPress deals with user passwords, database login credentials, etc. To protect these from appearing in logs or stack traces (which sometimes get copied into bug reports in forum threads and similar), using the #[\SensitiveParameter]
attribute for such variables should be explored.
As that attribute starts with a #
, which indicates a comment, it's safe to use this with older versions of PHP as well (similar to the #[\AllowDynamicProperties]
attribute that is already used in Core).
Change History (9)
#2
@
11 months ago
Thanks for this one @TobiasBg! I also support this change. Would you be able to create a first pass PR for this?
This ticket was mentioned in Slack in #core by costdev. View the logs.
10 months ago
#4
@
10 months ago
- Milestone changed from 6.2 to Future Release
This ticket was discussed during the bug scrub. As a patch hasn't been submitted yet, and 6.2 Beta 1 is only a few days away, I'm moving this ticket to the Future Release milestone.
Additional Props: @audrasjb @petitphp
#5
@
9 months ago
Started working on a patch for this ticket and I encounter a small quirk.
To make this change compatible with PHP7.x and below we'll need to use a custom formatting and include the attribute on its own line or this will cause a Fatal error since it's interpreted as a comment and discard the end of the line (see the 3v4l.org demo).
Example for the wpdb
constructor we could end up with :
public function __construct(
$dbuser,
#[\SensitiveParameter] <-- should be on its own line
$dbpassword,
$dbname,
$dbhost
) {
This looks ok to me, but I wanted to have other opinions before putting up a patch.
#6
@
9 months ago
That's fine, we do similar for function calls with multi-line parameters. Does this trigger a coding standards violation? If so we can put forward the case to change that.
#7
@
9 months ago
That's fine
Great! I'll finish the work on the patch. Thanks.
Does this trigger a coding standards violation?
It doesn't on my local machine when running the linter.
This ticket was mentioned in PR #4147 on WordPress/wordpress-develop by @petitphp.
9 months ago
#8
- Keywords has-patch added; needs-patch removed
Mark credentials with the new SensitiveParameter
attribute available in PHP 8.2 to hide them in stack traces or log.
Trac ticket: https://core.trac.wordpress.org/ticket/57304
I fully support this proposal.
Note for the implementation: the attribute can only be added to individual parameters in function declarations, so can not be applied to global or function local variables.
Related RFC: https://wiki.php.net/rfc/redact_parameters_in_back_traces
Side-note: we may need to verify that existing tooling, like the documentation generation tooling and WordPressCS, are able to deal with attributes for function parameters in function declarations. If not, those will need to be updated.