Skip to content

Commit 13c5c3f

Browse files
author
Kapil Borle
committed
Fix AvoidUsingPlainTextForPassword correction extent
Handles the following cases * [string] [Parameter(ValueFromPipeline)] $Password * [Parameter(ValueFromPipeline)] $Password * [Parameter(ValueFromPipeline)] [string[]] $Password * [string] <#whatever#> $Password
1 parent d85e978 commit 13c5c3f

File tree

3 files changed

+71
-13
lines changed

3 files changed

+71
-13
lines changed

Rules/AvoidUsingPlainTextForPassword.cs

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
using System.ComponentModel.Composition;
1818
using System.Globalization;
1919
using System.Reflection;
20+
using System.Text;
2021

2122
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
2223
{
@@ -71,21 +72,53 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
7172

7273
private List<CorrectionExtent> GetCorrectionExtent(ParameterAst paramAst)
7374
{
74-
IScriptExtent ext = paramAst.Extent;
75-
var corrections = new List<CorrectionExtent>();
76-
string correctionText = string.Format("[SecureString] {0}", paramAst.Name.Extent.Text);
77-
string description = string.Format("Set {0} type to SecureString", paramAst.Name.Extent.Text);
75+
//Find the parameter type extent and replace that with secure string
76+
IScriptExtent extent;
77+
var typeAttributeAst = GetTypeAttributeAst(paramAst);
78+
var corrections = new List<CorrectionExtent>();
79+
string correctionText;
80+
if (typeAttributeAst == null)
81+
{
82+
// cannot find any type attribute
83+
extent = paramAst.Name.Extent;
84+
correctionText = string.Format("[SecureString] {0}", paramAst.Name.Extent.Text);
85+
}
86+
else
87+
{
88+
// replace only the existing type with [SecureString]
89+
extent = typeAttributeAst.Extent;
90+
correctionText = typeAttributeAst.TypeName.IsArray ? "[SecureString[]]" : "[SecureString]";
91+
}
92+
string description = string.Format(
93+
CultureInfo.CurrentCulture,
94+
Strings.AvoidUsingPlainTextForPasswordCorrectionDescription,
95+
paramAst.Name.Extent.Text);
7896
corrections.Add(new CorrectionExtent(
79-
ext.StartLineNumber,
80-
ext.EndLineNumber,
81-
ext.StartColumnNumber,
82-
ext.EndColumnNumber,
83-
correctionText,
84-
ext.File,
97+
extent.StartLineNumber,
98+
extent.EndLineNumber,
99+
extent.StartColumnNumber,
100+
extent.EndColumnNumber,
101+
correctionText.ToString(),
102+
paramAst.Extent.File,
85103
description));
86104
return corrections;
87105
}
88106

107+
private TypeConstraintAst GetTypeAttributeAst(ParameterAst paramAst)
108+
{
109+
if (paramAst.Attributes != null)
110+
{
111+
foreach(var attr in paramAst.Attributes)
112+
{
113+
if (attr.GetType() == typeof(TypeConstraintAst))
114+
{
115+
return attr as TypeConstraintAst;
116+
}
117+
}
118+
}
119+
return null;
120+
}
121+
89122
/// <summary>
90123
/// GetName: Retrieves the name of this rule.
91124
/// </summary>

Tests/Rules/AvoidUsingPlainTextForPassword.ps1

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,25 @@
3838
}
3939

4040
function TestFunction($password, [System.Security.SecureString[]]$passphrases, [string]$passThru){
41-
}
41+
}
42+
43+
function TestFunction2
44+
{
45+
Param(
46+
[string] <#some dumb comment#> $password
47+
)
48+
}
49+
50+
function TestFunction3
51+
{
52+
Param(
53+
[string[]] $password
54+
)
55+
}
56+
57+
function TestFunction4
58+
{
59+
Param(
60+
[string] [Parameter(ValueFromPipeline)] $Password
61+
)
62+
}

Tests/Rules/AvoidUsingPlainTextForPassword.tests.ps1

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ Import-Module (Join-Path $directory "PSScriptAnalyzerTestHelper.psm1")
99

1010
Describe "AvoidUsingPlainTextForPassword" {
1111
Context "When there are violations" {
12-
It "has 3 avoid using plain text for password violations" {
13-
$violations.Count | Should Be 4
12+
$expectedNumViolations = 7
13+
It "has $expectedNumViolations violations" {
14+
$violations.Count | Should Be $expectedNumViolations
1415
}
1516

1617
It "suggests corrections" {
@@ -20,6 +21,9 @@ Describe "AvoidUsingPlainTextForPassword" {
2021
Test-CorrectionExtent $violationFilepath $violations[1] 1 '$passwordparam' '[SecureString] $passwordparam'
2122
Test-CorrectionExtent $violationFilepath $violations[2] 1 '$credential' '[SecureString] $credential'
2223
Test-CorrectionExtent $violationFilepath $violations[3] 1 '$password' '[SecureString] $password'
24+
Test-CorrectionExtent $violationFilepath $violations[4] 1 '[string]' '[SecureString]'
25+
Test-CorrectionExtent $violationFilepath $violations[5] 1 '[string[]]' '[SecureString[]]'
26+
Test-CorrectionExtent $violationFilepath $violations[6] 1 '[string]' '[SecureString]'
2327
}
2428

2529
It "has the correct violation message" {

0 commit comments

Comments
 (0)