Skip to content

Commit

Permalink
NET-685 S6418 Implement Equals Capability (#461)
Browse files Browse the repository at this point in the history 
Co-authored-by: Gregory Paidis <[email protected]>
  • Loading branch information
@alex-meseldzija-sonarsource @gregory-paidis-sonarsource @mary-georgiou-sonarsource
2 people authored and mary-georgiou-sonarsource committed Nov 27, 2024
1 parent 5e532ba commit 347297b
Show file tree
Hide file tree
Showing 7 changed files with 151 additions and 30 deletions.
6 changes: 6 additions & 0 deletions ...s/src/SonarAnalyzer.CSharp.Core/Syntax/Extensions/InvocationExpressionSyntaxExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ public static bool TryGetOperands(this InvocationExpressionSyntax invocation, ou
return left is not null && right is not null;
}

public static bool TryGetFirstArgument(this InvocationExpressionSyntax invocationExpression, out ArgumentSyntax firstArgument)
{
firstArgument = invocationExpression?.ArgumentList?.Arguments.FirstOrDefault();
return firstArgument is not null;
}

public static SyntaxToken? GetMethodCallIdentifier(this InvocationExpressionSyntax invocation) =>
invocation?.Expression.GetIdentifier();

Expand Down
71 changes: 64 additions & 7 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/Hotspots/DoNotHardcodeSecrets.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public sealed class DoNotHardcodeSecrets : DoNotHardcodeBase<SyntaxKind>
private const string DefaultSecretWords = """api[_\-]?key, auth, credential, secret, token""";
private const double DefaultRandomnessSensibility = 3;
private const double LanuageScoreIncrement = 0.3;
private const string EqualsName = nameof(string.Equals);

// https://docs.gitguardian.com/secrets-detection/secrets-detection-engine/detectors/generics/generic_high_entropy_secret#:~:text=Follow%20this%20regular%20expression
private static readonly Regex ValidationPattern = new(@"^[a-zA-Z0-9_.+/~$-]([a-zA-Z0-9_.+\/=~$-]|\\(?![ntr""])){14,1022}[a-zA-Z0-9_.+/=~$-]$", RegexOptions.None, RegexConstants.DefaultTimeout);
Expand Down Expand Up @@ -74,17 +75,35 @@ protected override void Initialize(SonarParametrizedAnalysisContext context)
&& FindRightHandSide(node) is { } rhs
&& rhs.FindStringConstant(c.SemanticModel) is { } secret
&& IsToken(secret)
&& ShouldRaise(identifier.ValueText, secret, out string message))
&& ShouldRaise(identifier.ValueText, secret, out var message))
{
c.ReportIssue(rule, rhs, message);
}
},
SyntaxKind.AddAssignmentExpression,
SyntaxKind.SimpleAssignmentExpression,
SyntaxKind.VariableDeclarator,
SyntaxKind.PropertyDeclaration,
SyntaxKind.GetAccessorDeclaration,
SyntaxKind.SetAccessorDeclaration);
SyntaxKind.AddAssignmentExpression,
SyntaxKind.SimpleAssignmentExpression,
SyntaxKind.VariableDeclarator,
SyntaxKind.PropertyDeclaration,
SyntaxKind.GetAccessorDeclaration,
SyntaxKind.SetAccessorDeclaration,
SyntaxKind.EqualsExpression);

c.RegisterNodeAction(c =>
{
var invocationExpression = (InvocationExpressionSyntax)c.Node;

if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression
&& memberAccessExpression.Name.Identifier.ValueText == EqualsName
&& invocationExpression.TryGetFirstArgument(out var firstArgument)
&& memberAccessExpression.IsMemberAccessOnKnownType(EqualsName, KnownType.System_String, c.SemanticModel)
&& GetIdentifierAndValue(memberAccessExpression.Expression, firstArgument, out var identifier, out var value)
&& value.FindStringConstant(c.SemanticModel) is { } secret
&& ShouldRaise(identifier.Value.ValueText, secret, out var message))
{
c.ReportIssue(rule, memberAccessExpression, message);
}
},
SyntaxKind.InvocationExpression);
});

context.RegisterCompilationAction(CheckWebConfig);
Expand Down Expand Up @@ -146,19 +165,57 @@ protected override IEnumerable<string> FindKeyWords(string variableName, string
{
AccessorDeclarationSyntax accessorDeclaration => accessorDeclaration.Parent.Parent.GetIdentifier(),
AssignmentExpressionSyntax assignmentExpression => assignmentExpression.Left.GetIdentifier(),
BinaryExpressionSyntax binaryExpression => GetBinaryExpressionIdentifier(binaryExpression),
_ => node.GetIdentifier()
};

private static SyntaxToken? GetBinaryExpressionIdentifier(BinaryExpressionSyntax node) =>
node switch
{
{ Left: IdentifierNameSyntax identifierLeft } => identifierLeft.Identifier,
{ Right: IdentifierNameSyntax identifierRight } => identifierRight.Identifier,
_ => null
};

private static SyntaxNode? GetBinaryExpressionValue(BinaryExpressionSyntax node) =>
node switch
{
{ Left: IdentifierNameSyntax } => node.Right,
{ Right: IdentifierNameSyntax } => node.Left,
_ => null
};

private static SyntaxNode FindRightHandSide(SyntaxNode node) =>
node switch
{
AssignmentExpressionSyntax assignmentExpression => assignmentExpression.Right,
VariableDeclaratorSyntax variableDeclarator => variableDeclarator.Initializer?.Value,
PropertyDeclarationSyntax propertyDeclaration => propertyDeclaration.Initializer?.Value,
AccessorDeclarationSyntax accessorDeclaration => accessorDeclaration.ExpressionBody()?.Expression,
BinaryExpressionSyntax binaryExpression => GetBinaryExpressionValue(binaryExpression),
_ => null
};

private static bool GetIdentifierAndValue(ExpressionSyntax expression, ArgumentSyntax argument, out SyntaxToken? identifier, out LiteralExpressionSyntax value)
{
identifier = null;
value = null;
switch (expression)
{
case MemberAccessExpressionSyntax:
case IdentifierNameSyntax:
case InvocationExpressionSyntax:
identifier = expression.GetIdentifier();
value = argument.Expression as LiteralExpressionSyntax;
break;
case LiteralExpressionSyntax:
identifier = argument.Expression.GetIdentifier();
value = expression as LiteralExpressionSyntax;
break;
}
return identifier.HasValue && value is not null;
}

private bool IsToken(string value) =>
ShannonEntropy.Calculate(value) > RandomnessSensibility
&& ValidationPattern.SafeIsMatch(value)
Expand Down
19 changes: 2 additions & 17 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/UseStringIsNullOrEmpty.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ protected override void Initialize(SonarAnalysisContext context)

if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression
&& memberAccessExpression.Name.Identifier.ValueText == EqualsName
&& TryGetFirstArgument(invocationExpression, out var firstArgument)
&& IsStringEqualsMethod(memberAccessExpression, c.SemanticModel))
&& invocationExpression.TryGetFirstArgument(out var firstArgument)
&& memberAccessExpression.IsMemberAccessOnKnownType(EqualsName, KnownType.System_String, c.SemanticModel))
{
// x.Equals(value), where x is string.Empty, "" or const "", and value is some string
if (IsStringIdentifier(firstArgument.Expression, c.SemanticModel)
Expand All @@ -65,21 +65,6 @@ protected override void Initialize(SonarAnalysisContext context)
SyntaxKind.InvocationExpression);
}

private static bool TryGetFirstArgument(InvocationExpressionSyntax invocationExpression, out ArgumentSyntax firstArgument)
{
firstArgument = invocationExpression?.ArgumentList?.Arguments.FirstOrDefault();

return firstArgument != null;
}

private static bool IsStringEqualsMethod(MemberAccessExpressionSyntax memberAccessExpression, SemanticModel semanticModel)
{
var methodName = semanticModel.GetSymbolInfo(memberAccessExpression.Name);

return methodName.Symbol.IsInType(KnownType.System_String)
&& methodName.Symbol.Name == EqualsName;
}

private static bool IsStringIdentifier(ExpressionSyntax expression, SemanticModel semanticModel)
{
if (!(expression is IdentifierNameSyntax identifierNameExpression))
Expand Down
10 changes: 10 additions & 0 deletions analyzers/src/SonarAnalyzer.Core/Rules/Hotspots/DoNotHardcodeBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ protected static bool IsValidKeyword(string suffix)

protected void CheckWebConfig(SonarCompilationReportingContext context)
{
if (!IsEnabled(context.Options))
{
return;
}

foreach (var path in context.WebConfigFiles())
{
if (XmlHelper.ParseXDocument(File.ReadAllText(path)) is { } doc)
Expand All @@ -89,6 +94,11 @@ protected void CheckWebConfig(SonarCompilationReportingContext context)

protected void CheckAppSettings(SonarCompilationReportingContext context)
{
if (!IsEnabled(context.Options))
{
return;
}

foreach (var path in context.AppSettingsFiles())
{
if (JsonNode.FromString(File.ReadAllText(path)) is { } json)
Expand Down
8 changes: 4 additions & 4 deletions analyzers/tests/SonarAnalyzer.Test/Rules/Hotspots/DoNotHardcodeCredentialsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ public void DoNotHardcodeCredentials_CS_CustomValues_CaseInsensitive() =>

[TestMethod]
public void DoNotHardcodeCredentials_CS_WebConfig() =>
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new CS.DoNotHardcodeCredentials(), "WebConfig", "*.config");
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled), "WebConfig", "*.config");

[TestMethod]
public void DoNotHardcodeCredentials_CS_AppSettings() =>
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new CS.DoNotHardcodeCredentials(), "AppSettings", "*.json");
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new CS.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled), "AppSettings", "*.json");

[TestMethod]
public void DoNotHardcodeCredentials_VB_DefaultValues() =>
Expand All @@ -93,11 +93,11 @@ public void DoNotHardcodeCredentials_VB_CustomValues_CaseInsensitive() =>

[TestMethod]
public void DoNotHardcodeCredentials_VB_WebConfig() =>
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.VisualBasic, new VB.DoNotHardcodeCredentials(), "WebConfig", "*.config");
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.VisualBasic, new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled), "WebConfig", "*.config");

[TestMethod]
public void DoNotHardcodeCredentials_VB_AppSettings() =>
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.VisualBasic, new VB.DoNotHardcodeCredentials(), "AppSettings", "*.json");
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.VisualBasic, new VB.DoNotHardcodeCredentials(AnalyzerConfiguration.AlwaysEnabled), "AppSettings", "*.json");

[TestMethod]
public void DoNotHardcodeCredentials_ConfiguredCredentialsAreRead()
Expand Down
4 changes: 2 additions & 2 deletions analyzers/tests/SonarAnalyzer.Test/Rules/Hotspots/DoNotHardcodeSecretsTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ public void DoNotHardcodeSecrets_DefaultValues() =>
// TODO: Add snippet with parametrisation
[TestMethod]
public void DoNotHardcodeSecrets_WebConfig() =>
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new DoNotHardcodeSecrets(), "WebConfig", "*.config");
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new DoNotHardcodeSecrets(AnalyzerConfiguration.AlwaysEnabled), "WebConfig", "*.config");

[TestMethod]
public void DoNotHardcodeSecrets_AppSettings() =>
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new DoNotHardcodeSecrets(), "AppSettings", "*.json");
DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage.CSharp, new DoNotHardcodeSecrets(AnalyzerConfiguration.AlwaysEnabled), "AppSettings", "*.json");

private void DoNotHardcodeCredentials_ExternalFiles(AnalyzerLanguage language, DiagnosticAnalyzer analyzer, string testDirectory, string pattern)
{
Expand Down
63 changes: 63 additions & 0 deletions analyzers/tests/SonarAnalyzer.Test/TestCases/Hotspots/DoNotHardcodeSecrets.cs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;

public class Program
{
Expand Down Expand Up @@ -53,9 +54,71 @@ public void SeaShark7VariableDeclaration()

var c_auth = "a";
c_auth = c_auth + "rf6acB24J//1FZLRrKpjmBUYSnUX5CHlt/iD5vVVcgVuAIOB6hzcWjDnv16V6hDLevW0Qs4hKPbP1M4YfuDI16sZna1/VGRLkAbTk6xMPs4epH6A3ZqSyyI-H92y"; // Compliant does not compile to a constant

var authToken = "";
var shouldNotRaise = ":)";
if (authToken == "31bf3856ad364e35") {}// Noncompliant
else if ("b03f5f7f11d50a3a" == authToken) {} // Noncompliant
else if(shouldNotRaise == "31bf3856ad364e35") {} // Compliant
else if("31bf3856ad364e35" == shouldNotRaise) {} // Compliant
if ("31bf3856ad364e35".Equals(authToken)){ } // Noncompliant
if (authToken.Equals("31bf3856ad364e35")) { } // Noncompliant
if ("31bf3856ad364e35".Equals(shouldNotRaise)){ } // Compliant
if (shouldNotRaise.Equals("31bf3856ad364e35")) { } // Compliant

"AuthToken".Equals("31bf3856ad364e35"); // Compliant
authToken.Equals(shouldNotRaise); // Compliant
shouldNotRaise.Equals(null); // Compliant
"AuthToken".Equals(null); // Compliant

"31bf3856ad364e35".Equals(authToken, StringComparison.CurrentCulture); // Noncompliant
authToken.Equals("31bf3856ad364e35", StringComparison.CurrentCulture); // Noncompliant

var bar = new Bar();
var foo = new Foo();

if (bar.test.test.ShouldNotRaise.Equals("31bf3856ad364e35")) { } // Compliant
if("31bf3856ad364e35".Equals(bar.test.test.ShouldNotRaise)) { } // Compliant
if (bar.test.test.Token.Equals("31bf3856ad364e35")) { } // Noncompliant
if("31bf3856ad364e35".Equals(bar.test.test.Token)) { } // Noncompliant

if (foo.GetToken().Equals("31bf3856ad364e35")) { } // Compliant not string.Equals
if (foo.GetAuthToken().Equals("31bf3856ad364e35")) { } // Noncompliant
if ("31bf3856ad364e35".Equals(foo.GetToken())) { } // Noncompliant
if (foo.GetToken().test.Equals("31bf3856ad364e35")) { } // Compliant
if ("31bf3856ad364e35".Equals(foo.GetToken().test)) { } // Compliant

string.Equals("31bf3856ad364e35", authToken); // Compliant FN
string.Equals(authToken, "31bf3856ad364e35"); // Compliant FN
string.Equals("31bf3856ad364e35", authToken, StringComparison.CurrentCulture); // Compliant FN
string.Equals(comparisonType: StringComparison.CurrentCulture, a: "31bf3856ad364e35", b: authToken); // Compliant FN
StringComparer.InvariantCulture.Equals("31bf3856ad364e35", authToken); // Compliant FN
StringComparer.InvariantCulture.Equals(authToken, "31bf3856ad364e35"); // Compliant FN
EqualityComparer<string>.Default.Equals("31bf3856ad364e35", authToken); // Compliant FN
EqualityComparer<string>.Default.Equals(authToken, "31bf3856ad364e35"); // Compliant FN
if ("31bf3856ad364e35" is "authToken") { } // Compliant FN
if("authToken" is "31bf3856ad364e35") { } // Compliant FN
switch (authToken)
{
case "31bf3856ad364e35": // Compliant FN
break;
}
}
}

public class Foo
{
public Bar GetToken() => new Bar();
public string GetAuthToken() => "31bf3856ad364e35";
}

public class Bar
{
public Bar test = new Bar();
public string ShouldNotRaise;
public string Token;
}

public class Scaffold
{
public string key { get; set; }
Expand Down

0 comments on commit 347297b

Please sign in to comment.