Skip to content

Commit

Permalink
Create rule S5344: Passwords should not be stored in plain-text or wi…
Browse files Browse the repository at this point in the history
…th a fast hashing algorithm
  • Loading branch information
gregory-paidis-sonarsource committed May 16, 2024
1 parent b04d8a0 commit 4e8d69a
Show file tree
Hide file tree
Showing 12 changed files with 846 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"Issues": [
{
"Id": "S5344",
"Message": "Use at least 100 000 iterations and a state-of-the-art digest algorithm here.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/NetFramework48/HashesShouldHaveUnpredictableSaltTest.cs#L21",
"Location": "Line 21 Position 13-64"
},
{
"Id": "S5344",
"Message": "Use at least 100 000 iterations and a state-of-the-art digest algorithm here.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/ManuallyAddedNoncompliantIssues.CS/NetFramework48/HashesShouldHaveUnpredictableSaltTest.cs#L26",
"Location": "Line 26 Position 13-63"
}
]
}
10 changes: 10 additions & 0 deletions analyzers/its/expected/Nancy/S5344-Nancy-net452.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"Issues": [
{
"Id": "S5344",
"Message": "Use at least 100 000 iterations and a state-of-the-art digest algorithm here.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Nancy/src/Nancy/Cryptography/PassphraseKeyGenerator.cs#L31",
"Location": "Line 31 Position 29-81"
}
]
}
10 changes: 10 additions & 0 deletions analyzers/its/expected/Nancy/S5344-Nancy-netstandard2.0.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"Issues": [
{
"Id": "S5344",
"Message": "Use at least 100 000 iterations and a state-of-the-art digest algorithm here.",
"Uri": "https://github.com/SonarSource/sonar-dotnet/blob/master/analyzers/its/Projects/Nancy/src/Nancy/Cryptography/PassphraseKeyGenerator.cs#L31",
"Location": "Line 31 Position 29-81"
}
]
}
317 changes: 317 additions & 0 deletions analyzers/rspec/cs/S5344.html

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions analyzers/rspec/cs/S5344.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"title": "Passwords should not be stored in plaintext or with a fast hashing algorithm",
"type": "VULNERABILITY",
"code": {
"impacts": {
"SECURITY": "HIGH"
},
"attribute": "TRUSTWORTHY"
},
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "30min"
},
"tags": [
"cwe",
"spring"
],
"defaultSeverity": "Critical",
"ruleSpecification": "RSPEC-5344",
"sqKey": "S5344",
"scope": "Main",
"securityStandards": {
"CWE": [
256,
916
],
"OWASP": [
"A3"
],
"OWASP Top 10 2021": [
"A2",
"A4"
],
"PCI DSS 3.2": [
"6.5.3"
],
"PCI DSS 4.0": [
"6.2.4"
],
"ASVS 4.0": [
"2.10.3",
"2.4.1",
"2.4.2",
"2.4.3",
"2.4.4",
"2.4.5"
],
"STIG ASD 2023-06-08": [
"V-222542"
]
},
"quickfix": "unknown"
}
1 change: 1 addition & 0 deletions analyzers/rspec/cs/Sonar_way_profile.json
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
"S5042",
"S5122",
"S5332",
"S5344",
"S5443",
"S5445",
"S5542",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/*
* SonarAnalyzer for .NET
* Copyright (C) 2015-2024 SonarSource SA
* mailto: contact AT sonarsource DOT com
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using SonarAnalyzer.Helpers.Trackers;

namespace SonarAnalyzer.Rules.CSharp;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class PasswordsShouldBeStoredCorrectly : SonarDiagnosticAnalyzer
{
private const string DiagnosticId = "S5344";
private const string MessageFormat = "{0}";
private const string UseMoreIterationsMessageFormat = "Use at least 100,000 iterations here.";
private const int IterationCountThreshold = 100_000;

private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, MessageFormat);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

protected override void Initialize(SonarAnalysisContext context)
{
NetCore(context);
Rfc2898DeriveBytes(context);
BouncyCastle(context);
}

private static void NetCore(SonarAnalysisContext context)
{
var propertyTracker = CSharpFacade.Instance.Tracker.PropertyAccess;
Track(
propertyTracker,
context,
UseMoreIterationsMessageFormat,
propertyTracker.MatchSetter(),
propertyTracker.MatchProperty(new MemberDescriptor(KnownType.Microsoft_AspNetCore_Identity_PasswordsHasherOptions, "IterationCount")),
x => HasFewIterations(x, propertyTracker));
Track(
propertyTracker,
context,
"Identity v2 uses only 1000 iterations. Consider changing to identity V3.",
propertyTracker.MatchSetter(),
propertyTracker.MatchProperty(new MemberDescriptor(KnownType.Microsoft_AspNetCore_Identity_PasswordsHasherOptions, "CompatibilityMode")),
x => propertyTracker.AssignedValue(x) is int mode && mode == 0); // PasswordHasherCompatibilityMode.IdentityV2 results to 0

var argumentTracker = CSharpFacade.Instance.Tracker.Argument;
Track(
argumentTracker,
context,
UseMoreIterationsMessageFormat,
argumentTracker.MatchArgument(ArgumentDescriptor.MethodInvocation(KnownType.Microsoft_AspNetCore_Cryptography_KeyDerivation_KeyDerivation, "Pbkdf2", "iterationCount", 3)),
x => ArgumentLessThan(x, IterationCountThreshold));
}

private static void Rfc2898DeriveBytes(SonarAnalysisContext context)
{
// Raise when hashAlgorithm is present
var argumentTracker = CSharpFacade.Instance.Tracker.Argument;
// Exclude the constructors that have a hashAlgorithm parameter
var constructorArgument = ArgumentDescriptor.ConstructorInvocation(
ctor => ctor.ContainingType.Is(KnownType.System_Security_Cryptography_Rfc2898DeriveBytes) && ctor.Parameters.Any(x => x.Name == "hashAlgorithm"),
(methodName, comparison) => string.Compare(methodName, "Rfc2898DeriveBytes", comparison) == 0,
null,
x => x.Name == "iterations",
null,
null);
var invocationArgument = ArgumentDescriptor.MethodInvocation(KnownType.System_Security_Cryptography_Rfc2898DeriveBytes, "Pbkdf2", "iterations", x => x is 2 or 3);
Track(
argumentTracker,
context,
UseMoreIterationsMessageFormat,
argumentTracker.Or(
argumentTracker.MatchArgument(constructorArgument),
argumentTracker.MatchArgument(invocationArgument)),
x => ArgumentLessThan(x, IterationCountThreshold));

// Raise when hashAlgorithm is NOT present
var objectCreationTracker = CSharpFacade.Instance.Tracker.ObjectCreation;
Track(
objectCreationTracker,
context,
"Use at least 100,000 iterations and a state-of-the-art digest algorithm here.",
objectCreationTracker.MatchConstructor(KnownType.System_Security_Cryptography_Rfc2898DeriveBytes),
x => x.InvokedConstructorSymbol.Value.Parameters.All(x => x.Name != "hashAlgorithm"));
}

private static void BouncyCastle(SonarAnalysisContext context)
{
var tracker = CSharpFacade.Instance.Tracker.Argument;

Track(
tracker,
context,
"Use a cost factor of at least 12 here.",
tracker.Or(
tracker.MatchArgument(ArgumentDescriptor.MethodInvocation(KnownType.Org_BouncyCastle_Crypto_Generators_OpenBsdBCrypt, "Generate", "cost", x => x is 2 or 3)),
tracker.MatchArgument(ArgumentDescriptor.MethodInvocation(KnownType.Org_BouncyCastle_Crypto_Generators_BCrypt, "Generate", "cost", 2))),
x => ArgumentLessThan(x, 12));

Track(
tracker,
context,
UseMoreIterationsMessageFormat,
tracker.MatchArgument(ArgumentDescriptor.MethodInvocation(KnownType.Org_BouncyCastle_Crypto_PbeParametersGenerator, "Init", "iterationCount", 2)),
x => ArgumentLessThan(x, IterationCountThreshold));
}

private static bool HasFewIterations(PropertyAccessContext context, PropertyAccessTracker<SyntaxKind> tracker) =>
tracker.AssignedValue(context) is int iterationCount
&& iterationCount < IterationCountThreshold;

private static bool ArgumentLessThan(ArgumentContext context, int threshold) =>
context.SemanticModel.GetConstantValue(((ArgumentSyntax)context.Node).Expression) is { HasValue: true, Value: int value }
&& value < threshold;

private static void Track<TContext>(SyntaxTrackerBase<SyntaxKind, TContext> tracker,
SonarAnalysisContext context,
string message,
params SyntaxTrackerBase<SyntaxKind, TContext>.Condition[] conditions) where TContext : SyntaxBaseContext =>
tracker.Track(new(context, AnalyzerConfiguration.AlwaysEnabled, Rule), [message], conditions);
}
5 changes: 5 additions & 0 deletions analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public sealed partial class KnownType
public static readonly KnownType Microsoft_AspNetCore_Components_RouteAttribute = new("Microsoft.AspNetCore.Components.RouteAttribute");
public static readonly KnownType Microsoft_AspNetCore_Components_SupplyParameterFromQueryAttribute = new("Microsoft.AspNetCore.Components.SupplyParameterFromQueryAttribute");
public static readonly KnownType Microsoft_AspNetCore_Cors_Infrastructure_CorsPolicyBuilder = new("Microsoft.AspNetCore.Cors.Infrastructure.CorsPolicyBuilder");
public static readonly KnownType Microsoft_AspNetCore_Cryptography_KeyDerivation_KeyDerivation = new("Microsoft.AspNetCore.Cryptography.KeyDerivation.KeyDerivation");
public static readonly KnownType Microsoft_AspNetCore_Hosting_HostingEnvironmentExtensions = new("Microsoft.AspNetCore.Hosting.HostingEnvironmentExtensions");
public static readonly KnownType Microsoft_AspNetCore_Hosting_WebHostBuilderExtensions = new("Microsoft.AspNetCore.Hosting.WebHostBuilderExtensions");
public static readonly KnownType Microsoft_AspNetCore_Http_CookieOptions = new("Microsoft.AspNetCore.Http.CookieOptions");
Expand All @@ -73,6 +74,7 @@ public sealed partial class KnownType
public static readonly KnownType Microsoft_AspNetCore_Http_IResponseCookies = new("Microsoft.AspNetCore.Http.IResponseCookies");
public static readonly KnownType Microsoft_AspNetCore_Http_IResult = new("Microsoft.AspNetCore.Http.IResult");
public static readonly KnownType Microsoft_AspNetCore_Http_Results = new("Microsoft.AspNetCore.Http.Results");
public static readonly KnownType Microsoft_AspNetCore_Identity_PasswordsHasherOptions = new("Microsoft.AspNetCore.Identity.PasswordHasherOptions");
public static readonly KnownType Microsoft_AspNetCore_Mvc_AcceptVerbsAttribute = new("Microsoft.AspNetCore.Mvc.AcceptVerbsAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_ApiControllerAttribute = new("Microsoft.AspNetCore.Mvc.ApiControllerAttribute");
public static readonly KnownType Microsoft_AspNetCore_Mvc_ApiConventionMethodAttribute = new("Microsoft.AspNetCore.Mvc.ApiConventionMethodAttribute");
Expand Down Expand Up @@ -208,9 +210,12 @@ public sealed partial class KnownType
public static readonly KnownType Org_BouncyCastle_Asn1_X9_ECNamedCurveTable = new("Org.BouncyCastle.Asn1.X9.ECNamedCurveTable");
public static readonly KnownType Org_BouncyCastle_Asn1_X9_X962NamedCurves = new("Org.BouncyCastle.Asn1.X9.X962NamedCurves");
public static readonly KnownType Org_BouncyCastle_Crypto_Engines_AesFastEngine = new("Org.BouncyCastle.Crypto.Engines.AesFastEngine");
public static readonly KnownType Org_BouncyCastle_Crypto_Generators_BCrypt = new("Org.BouncyCastle.Crypto.Generators.BCrypt");
public static readonly KnownType Org_BouncyCastle_Crypto_Generators_DHParametersGenerator = new("Org.BouncyCastle.Crypto.Generators.DHParametersGenerator");
public static readonly KnownType Org_BouncyCastle_Crypto_Generators_OpenBsdBCrypt = new("Org.BouncyCastle.Crypto.Generators.OpenBsdBCrypt");
public static readonly KnownType Org_BouncyCastle_Crypto_Generators_DsaParametersGenerator = new("Org.BouncyCastle.Crypto.Generators.DsaParametersGenerator");
public static readonly KnownType Org_BouncyCastle_Crypto_Parameters_RsaKeyGenerationParameters = new("Org.BouncyCastle.Crypto.Parameters.RsaKeyGenerationParameters");
public static readonly KnownType Org_BouncyCastle_Crypto_PbeParametersGenerator = new("Org.BouncyCastle.Crypto.PbeParametersGenerator");
public static readonly KnownType Serilog_Events_LogEventLevel = new("Serilog.Events.LogEventLevel");
public static readonly KnownType Serilog_ILogger = new("Serilog.ILogger");
public static readonly KnownType Serilog_LoggerConfiguration = new("Serilog.LoggerConfiguration");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5268,7 +5268,7 @@ internal static class RuleTypeMappingCS
// ["S5341"],
// ["S5342"],
// ["S5343"],
// ["S5344"],
["S5344"] = "VULNERABILITY",
// ["S5345"],
// ["S5346"],
// ["S5347"],
Expand Down
Loading

0 comments on commit 4e8d69a

Please sign in to comment.