PowerShell / PSScriptAnalyzer

Download ScriptAnalyzer from PowerShellGallery
https://www.powershellgallery.com/packages/PSScriptAnalyzer/
MIT License
1.8k stars 365 forks source link

Any variable containing the word "credit" triggers PSAvoidUsingPlainTextForPassword #1886

Open BrianL-STCU opened 1 year ago

BrianL-STCU commented 1 year ago

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
bergmeister commented 1 year ago

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.

BrianL-STCU commented 1 year ago

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

SydneyhSmith commented 1 year ago

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

BrianL-STCU commented 1 year ago

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.

bergmeister commented 1 year ago

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

BrianL-STCU commented 1 year ago

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.