PSScriptAnalyzer icon indicating copy to clipboard operation
PSScriptAnalyzer copied to clipboard

Any variable containing the word "credit" triggers PSAvoidUsingPlainTextForPassword

Open BrianL-STCU opened this issue 2 years ago • 6 comments

Steps to reproduce

Invoke-ScriptAnalyzer -IncludeSuppressed -ScriptDefinition 'Param([string] $creditor = ""); Write-Information $creditor'

Expected behavior

(no output)

Actual behavior

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSAvoidUsingPlainTextForPassword    Warning                 1     Parameter '$creditor' should not use String type but either
                                                                  SecureString or PSCredential, otherwise it increases the
                                                                  chance to to expose this sensitive information.

Environment data

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.3.2
PSEdition                      Core
GitCommitId                    7.3.2
OS                             Microsoft Windows 10.0.19044
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0


> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }
1.21.0

BrianL-STCU avatar Jan 31 '23 21:01 BrianL-STCU

Currently it checks whether the variable name contains one of those words, one of which is cred https://github.com/PowerShell/PSScriptAnalyzer/blob/f77acdf4cfed22b4b5bd106ac07951991ce824b7/Rules/AvoidUsingPlainTextForPassword.cs#L34 The logic could be tweak to specifically exclude credit or be more specific.

bergmeister avatar Feb 01 '23 17:02 bergmeister

"Cred" can show up in "creditworthiness", "accreditations', "credibility", "credo", &c, so that may be too broad.

BrianL-STCU avatar Feb 02 '23 00:02 BrianL-STCU

Thanks cred is chosen because it is the most used abbreviation... we would rather see a PR for a more specific compare rather than a list of words to exclude

SydneyhSmith avatar Feb 14 '23 23:02 SydneyhSmith

I didn't mean to imply that those words should be excluded, just that cred matches too many irrelevant words. If cred can be narrowed to cred and creds, or just as a suffix, maybe that would work? As it is, this is going to be a rule I'll have to exclude entirely.

BrianL-STCU avatar Feb 15 '23 18:02 BrianL-STCU

This is marked as an improvement but there will always be false positives, which is what the suppression feature is catering for. I suggest you check this out and do that instead of excluding rule but entirely up to you https://learn.microsoft.com/en-us/powershell/utility-modules/psscriptanalyzer/using-scriptanalyzer?view=ps-modules#suppressing-rules

bergmeister avatar Feb 16 '23 18:02 bergmeister

Yes, I excluding/suppressing is what I meant, but I'd like to keep using it. It's a good rule that has simply overextended its reach. Working in finance, cred is just going to match too many credit card-related fields.

BrianL-STCU avatar Feb 17 '23 01:02 BrianL-STCU