diff --git a/Rules/AvoidUsingPlainTextForPassword.cs b/Rules/AvoidUsingPlainTextForPassword.cs index f44585afc..6d877e400 100644 --- a/Rules/AvoidUsingPlainTextForPassword.cs +++ b/Rules/AvoidUsingPlainTextForPassword.cs @@ -64,37 +64,43 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } - private List GetCorrectionExtent(ParameterAst paramAst) + private IEnumerable GetCorrectionExtent(ParameterAst paramAst) { //Find the parameter type extent and replace that with secure string IScriptExtent extent; var typeAttributeAst = GetTypeAttributeAst(paramAst); var corrections = new List(); string correctionText; - if (typeAttributeAst == null) - { - // cannot find any type attribute - extent = paramAst.Name.Extent; - correctionText = string.Format("[SecureString] {0}", paramAst.Name.Extent.Text); - } - else + + foreach (string correctionType in new[] { "SecureString", "PSCredential" }) { - // replace only the existing type with [SecureString] - extent = typeAttributeAst.Extent; - correctionText = typeAttributeAst.TypeName.IsArray ? "[SecureString[]]" : "[SecureString]"; + if (typeAttributeAst == null) + { + // cannot find any type attribute + extent = paramAst.Name.Extent; + correctionText = $"[{correctionType}] {paramAst.Name.Extent.Text}"; + } + else + { + // replace only the existing type with [SecureString] + extent = typeAttributeAst.Extent; + correctionText = typeAttributeAst.TypeName.IsArray ? $"[{correctionType}[]]" : $"[{correctionType}]"; + } + string description = string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidUsingPlainTextForPasswordCorrectionDescription, + paramAst.Name.Extent.Text, + correctionType); + corrections.Add(new CorrectionExtent( + extent.StartLineNumber, + extent.EndLineNumber, + extent.StartColumnNumber, + extent.EndColumnNumber, + correctionText.ToString(), + paramAst.Extent.File, + description)); } - string description = string.Format( - CultureInfo.CurrentCulture, - Strings.AvoidUsingPlainTextForPasswordCorrectionDescription, - paramAst.Name.Extent.Text); - corrections.Add(new CorrectionExtent( - extent.StartLineNumber, - extent.EndLineNumber, - extent.StartColumnNumber, - extent.EndColumnNumber, - correctionText.ToString(), - paramAst.Extent.File, - description)); + return corrections; } diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 9fead171c..38ea09e8b 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -19,7 +19,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer { // class via a tool like ResGen or Visual Studio. // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. - [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "16.0.0.0")] + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")] [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Strings { @@ -1042,7 +1042,7 @@ internal static string AvoidUsingPlainTextForPasswordCommonName { } /// - /// Looks up a localized string similar to Set {0} type to SecureString. + /// Looks up a localized string similar to Set {0} type to {1}. /// internal static string AvoidUsingPlainTextForPasswordCorrectionDescription { get { @@ -1060,7 +1060,7 @@ internal static string AvoidUsingPlainTextForPasswordDescription { } /// - /// Looks up a localized string similar to Parameter '{0}' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.. + /// Looks up a localized string similar to Parameter '{0}' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to to expose this sensitive information.. /// internal static string AvoidUsingPlainTextForPasswordError { get { diff --git a/Rules/Strings.resx b/Rules/Strings.resx index da1032671..8872d2911 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -292,7 +292,7 @@ Password parameters that take in plaintext will expose passwords and compromise the security of your system. - Parameter '{0}' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information. + Parameter '{0}' should not use String type but either SecureString or PSCredential, otherwise it increases the chance to to expose this sensitive information. Avoid Using Plain Text For Password Parameter @@ -778,7 +778,7 @@ Replace {0} with {1} - Set {0} type to SecureString + Set {0} type to {1} Add {0} = {1} to the module manifest @@ -1164,4 +1164,4 @@ AvoidMultipleTypeAttributes - + \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 b/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 index 3fc6fda71..340893f96 100644 --- a/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 +++ b/Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1 @@ -2,7 +2,7 @@ # Licensed under the MIT License. BeforeAll { - $violationMessage = [regex]::Escape("Parameter '`$password' should use SecureString, otherwise this will expose sensitive information. See ConvertTo-SecureString for more information.") + $violationMessage = [regex]::Escape("Parameter '`$password' should not use String type but either ") $violationName = "PSAvoidUsingPlainTextForPassword" $violationFilepath = Join-Path $PSScriptRoot 'AvoidUsingPlainTextForPassword.ps1' $violations = Invoke-ScriptAnalyzer $violationFilepath | Where-Object { $_.RuleName -eq $violationName } @@ -17,15 +17,15 @@ Describe "AvoidUsingPlainTextForPassword" { } It "suggests corrections" { - Test-CorrectionExtent $violationFilepath $violations[0] 1 '$passphrases' '[SecureString] $passphrases' + Test-CorrectionExtent $violationFilepath $violations[0] 2 '$passphrases' '[SecureString] $passphrases' $violations[0].SuggestedCorrections[0].Description | Should -Be 'Set $passphrases type to SecureString' - Test-CorrectionExtent $violationFilepath $violations[1] 1 '$passwordparam' '[SecureString] $passwordparam' - Test-CorrectionExtent $violationFilepath $violations[2] 1 '$credential' '[SecureString] $credential' - Test-CorrectionExtent $violationFilepath $violations[3] 1 '$password' '[SecureString] $password' - Test-CorrectionExtent $violationFilepath $violations[4] 1 '[string]' '[SecureString]' - Test-CorrectionExtent $violationFilepath $violations[5] 1 '[string[]]' '[SecureString[]]' - Test-CorrectionExtent $violationFilepath $violations[6] 1 '[string]' '[SecureString]' + Test-CorrectionExtent $violationFilepath $violations[1] 2 '$passwordparam' '[SecureString] $passwordparam' + Test-CorrectionExtent $violationFilepath $violations[2] 2 '$credential' '[SecureString] $credential' + Test-CorrectionExtent $violationFilepath $violations[3] 2 '$password' '[SecureString] $password' + Test-CorrectionExtent $violationFilepath $violations[4] 2 '[string]' '[SecureString]' + Test-CorrectionExtent $violationFilepath $violations[5] 2 '[string[]]' '[SecureString[]]' + Test-CorrectionExtent $violationFilepath $violations[6] 2 '[string]' '[SecureString]' } It "has the correct violation message" {