diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/PasswordsShouldBeStoredCorrectly.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/PasswordsShouldBeStoredCorrectly.cs index 8757742bf01..9da7ede64dd 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/PasswordsShouldBeStoredCorrectly.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/PasswordsShouldBeStoredCorrectly.cs @@ -36,12 +36,13 @@ public sealed class PasswordsShouldBeStoredCorrectly : SonarDiagnosticAnalyzer protected override void Initialize(SonarAnalysisContext context) { - NetCore(context); + AspNetCore(context); + AspNetFramework(context); Rfc2898DeriveBytes(context); BouncyCastle(context); } - private static void NetCore(SonarAnalysisContext context) + private static void AspNetCore(SonarAnalysisContext context) { var propertyTracker = CSharpFacade.Instance.Tracker.PropertyAccess; Track( @@ -49,15 +50,15 @@ private static void NetCore(SonarAnalysisContext context) context, UseMoreIterationsMessageFormat, propertyTracker.MatchSetter(), - propertyTracker.MatchProperty(new MemberDescriptor(KnownType.Microsoft_AspNetCore_Identity_PasswordsHasherOptions, "IterationCount")), + propertyTracker.MatchProperty(new MemberDescriptor(KnownType.Microsoft_AspNetCore_Identity_PasswordHasherOptions, "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 + propertyTracker.MatchProperty(new MemberDescriptor(KnownType.Microsoft_AspNetCore_Identity_PasswordHasherOptions, "CompatibilityMode")), + x => propertyTracker.AssignedValue(x) is 0); // PasswordHasherCompatibilityMode.IdentityV2 results to 0 var argumentTracker = CSharpFacade.Instance.Tracker.Argument; Track( @@ -68,6 +69,16 @@ private static void NetCore(SonarAnalysisContext context) x => ArgumentLessThan(x, IterationCountThreshold)); } + private static void AspNetFramework(SonarAnalysisContext context) + { + var tracker = CSharpFacade.Instance.Tracker.ObjectCreation; + Track( + tracker, + context, + "PasswordHasher does not support state-of-the-art parameters. Use Rfc2898DeriveBytes instead.", + tracker.WhenDerivesFrom(KnownType.Microsoft_AspNet_Identity_PasswordHasherOptions)); + } + private static void Rfc2898DeriveBytes(SonarAnalysisContext context) { // Raise when hashAlgorithm is present @@ -98,6 +109,15 @@ private static void Rfc2898DeriveBytes(SonarAnalysisContext 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")); + + var propertyTracker = CSharpFacade.Instance.Tracker.PropertyAccess; + Track( + propertyTracker, + context, + UseMoreIterationsMessageFormat, + propertyTracker.MatchSetter(), + propertyTracker.MatchProperty(new MemberDescriptor(KnownType.System_Security_Cryptography_Rfc2898DeriveBytes, "IterationCount")), + x => HasFewIterations(x, propertyTracker)); } private static void BouncyCastle(SonarAnalysisContext context) @@ -109,21 +129,36 @@ private static void BouncyCastle(SonarAnalysisContext context) 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))), + 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)), + tracker.MatchArgument( + ArgumentDescriptor.MethodInvocation(KnownType.Org_BouncyCastle_Crypto_PbeParametersGenerator, "Init", "iterationCount", 2)), x => ArgumentLessThan(x, IterationCountThreshold)); + + TrackSCrypt("N", 2, "Use a cost factor of at least 2 ^ 12 for N here.", 1 << 12); + TrackSCrypt("r", 3, "Use a memory factor of at least 8 for r here.", 8); + TrackSCrypt("dkLen", 5, "Use an output length of at least 32 for dkLen here.", 32); + + void TrackSCrypt(string argumentName, int argumentPosition, string diagnosticMessage, int threshold) => + Track( + tracker, + context, + diagnosticMessage, + tracker.MatchArgument(ArgumentDescriptor.MethodInvocation( + KnownType.Org_BouncyCastle_Crypto_Generators_SCrypt, "Generate", argumentName, argumentPosition)), + x => ArgumentLessThan(x, threshold)); } private static bool HasFewIterations(PropertyAccessContext context, PropertyAccessTracker tracker) => - tracker.AssignedValue(context) is int iterationCount - && iterationCount < IterationCountThreshold; + tracker.AssignedValue(context) is < IterationCountThreshold; private static bool ArgumentLessThan(ArgumentContext context, int threshold) => context.SemanticModel.GetConstantValue(((ArgumentSyntax)context.Node).Expression) is { HasValue: true, Value: int value } diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs index 226a7ad3889..f74d55552e6 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs @@ -53,6 +53,7 @@ public sealed partial class KnownType public static readonly KnownType log4net_ILog = new("log4net.ILog"); public static readonly KnownType log4net_LogManager = new("log4net.LogManager"); public static readonly KnownType log4net_Util_ILogExtensions = new("log4net.Util.ILogExtensions"); + public static readonly KnownType Microsoft_AspNet_Identity_PasswordHasherOptions = new("Microsoft.AspNet.Identity.PasswordHasherOptions"); public static readonly KnownType Microsoft_AspNet_SignalR_Hub = new("Microsoft.AspNet.SignalR.Hub"); public static readonly KnownType Microsoft_AspNetCore_Builder_DeveloperExceptionPageExtensions = new("Microsoft.AspNetCore.Builder.DeveloperExceptionPageExtensions"); public static readonly KnownType Microsoft_AspNetCore_Builder_DatabaseErrorPageExtensions = new("Microsoft.AspNetCore.Builder.DatabaseErrorPageExtensions"); @@ -74,7 +75,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_Identity_PasswordHasherOptions = 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"); @@ -211,6 +212,7 @@ public sealed partial class KnownType 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_SCrypt = new("Org.BouncyCastle.Crypto.Generators.SCrypt"); 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"); diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/PasswordsShouldBeStoredCorrectlyTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/PasswordsShouldBeStoredCorrectlyTest.cs index 30f11c6aaf0..7499c44da4b 100644 --- a/analyzers/tests/SonarAnalyzer.Test/Rules/PasswordsShouldBeStoredCorrectlyTest.cs +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/PasswordsShouldBeStoredCorrectlyTest.cs @@ -36,21 +36,51 @@ public class PasswordsShouldBeStoredCorrectlyTest .AddReferences([ AspNetCoreMetadataReference.MicrosoftExtensionsIdentityCore, AspNetCoreMetadataReference.MicrosoftAspNetCoreCryptographyKeyDerivation, - CoreMetadataReference.SystemSecurityCryptography, + ..MetadataReferenceFacade.SystemSecurityCryptography, ]) .Verify(); #endif + [TestMethod] + public void PasswordsShouldBeStoredCorrectly_CS_PasswordHasherOptions() => + Builder + .WithOptions(ParseOptionsHelper.FromCSharp9) + .AddReferences(NuGetMetadataReference.MicrosoftAspNetIdentity()) + .AddSnippet(""" + using Microsoft.AspNet.Identity; + + class Testcases + { + void Method() + { + var _ = new PasswordHasherOptions(); // Noncompliant {{PasswordHasher does not support state-of-the-art parameters. Use Rfc2898DeriveBytes instead.}} + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + PasswordHasherOptions x = new(); // Noncompliant + // ^^^^^ + _ = new PasswordHasherOptions() {}; // Noncompliant + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + _ = new PasswordHasherOptions { IterationCount = 42 }; // Noncompliant + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + _ = new Derived(); // Noncompliant + // ^^^^^^^^^^^^^ + } + } + + public class Derived: PasswordHasherOptions { } + """) + .Verify(); + [TestMethod] public void PasswordsShouldBeStoredCorrectly_CS_Rfc2898DeriveBytes() => Builder + .AddReferences(MetadataReferenceFacade.SystemSecurityCryptography) .AddSnippet(""" using System.Security.Cryptography; class Testcases { const int ITERATIONS = 100_000; - void Method(int iterations, byte[] bs) + void Method(int iterations, byte[] bs, HashAlgorithmName han) { new Rfc2898DeriveBytes("password", bs); // Noncompliant new Rfc2898DeriveBytes("password", 42); // Noncompliant @@ -60,28 +90,46 @@ void Method(int iterations, byte[] bs) // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ new Rfc2898DeriveBytes("password", bs, 42); // Noncompliant new Rfc2898DeriveBytes("password", 42, 42); // Noncompliant - new Rfc2898DeriveBytes(bs, bs, 42, HashAlgorithmName.MD5); // Noncompliant {{Use at least 100,000 iterations here.}} + new Rfc2898DeriveBytes(bs, bs, 42, han); // Noncompliant {{Use at least 100,000 iterations here.}} // ^^ - new Rfc2898DeriveBytes("password", bs, 42, HashAlgorithmName.MD5); // Noncompliant - new Rfc2898DeriveBytes("password", 42, 42, HashAlgorithmName.MD5); // Noncompliant + new Rfc2898DeriveBytes("password", bs, 42, han); // Noncompliant + new Rfc2898DeriveBytes("password", 42, 42, han); // Noncompliant new Rfc2898DeriveBytes(bs, bs, ITERATIONS); // Noncompliant new Rfc2898DeriveBytes("", bs, ITERATIONS); // Noncompliant new Rfc2898DeriveBytes("", 42, ITERATIONS); // Noncompliant // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - new Rfc2898DeriveBytes(bs, bs, iterations, HashAlgorithmName.SHA1); // Compliant - new Rfc2898DeriveBytes(bs, bs, ITERATIONS, HashAlgorithmName.SHA1); // Compliant - new Rfc2898DeriveBytes("", bs, ITERATIONS, HashAlgorithmName.SHA1); // Compliant - new Rfc2898DeriveBytes("", 42, ITERATIONS, HashAlgorithmName.SHA1); // Compliant + new Rfc2898DeriveBytes(bs, bs, iterations, han); // Compliant + new Rfc2898DeriveBytes(bs, bs, ITERATIONS, han); // Compliant + new Rfc2898DeriveBytes("", bs, ITERATIONS, han); // Compliant + new Rfc2898DeriveBytes("", 42, ITERATIONS, han); // Compliant + + var x = new Rfc2898DeriveBytes(bs, bs, ITERATIONS, han); + x.IterationCount = 1; // Noncompliant {{Use at least 100,000 iterations here.}} + // ^^^^^^^^^^^^^^^^ + + x.IterationCount = 100_042; // Compliant + + new Rfc2898DeriveBytes(bs, bs, ITERATIONS, han) + { + IterationCount = 42 // Noncompliant {{Use at least 100,000 iterations here.}} + // ^^^^^^^^^^^^^^ + }; + } + + void MakeItUnsafe(Rfc2898DeriveBytes password) + { + password.IterationCount = 1; // Noncompliant {{Use at least 100,000 iterations here.}} + // ^^^^^^^^^^^^^^^^^^^^^^^ } } """) - .AddReferences(MetadataReferenceFacade.SystemSecurityCryptography) .Verify(); [TestMethod] public void PasswordsShouldBeStoredCorrectly_CS_BouncyCastle_Generate() => Builder + .AddReferences(NuGetMetadataReference.BouncyCastle()) .AddSnippet(""" using Org.BouncyCastle.Crypto.Generators; @@ -118,12 +166,12 @@ void Method(char[] cs, byte[] bs) } } """) - .AddReferences(NuGetMetadataReference.BouncyCastle()) .Verify(); [TestMethod] public void PasswordsShouldBeStoredCorrectly_CS_BouncyCastle_Init() => Builder + .AddReferences(NuGetMetadataReference.BouncyCastle()) .AddSnippet(""" using Org.BouncyCastle.Crypto; using Org.BouncyCastle.Crypto.Generators; @@ -144,6 +192,34 @@ void Method(byte[] bs, PbeParametersGenerator baseGen, Pkcs5S2ParametersGenerato } } """) + .Verify(); + + [TestMethod] + public void PasswordsShouldBeStoredCorrectly_CS_BouncyCastle_Generate_SCrypt() => + Builder .AddReferences(NuGetMetadataReference.BouncyCastle()) + .AddSnippet(""" + using Org.BouncyCastle.Crypto.Generators; + + class Testcases + { + void Method(byte[] bs, int p) + { + SCrypt.Generate(bs, bs, 1 << 12, 8, p, 32); // Compliant + + SCrypt.Generate(bs, bs, 1 << 11, 42, p, 42); // Noncompliant {{Use a cost factor of at least 2 ^ 12 for N here.}} + // ^^^^^^^ + SCrypt.Generate(bs, bs, 1 << 12, 7, p, 42); // Noncompliant {{Use a memory factor of at least 8 for r here.}} + // ^ + SCrypt.Generate(bs, bs, 1 << 12, 42, p, 31); // Noncompliant {{Use an output length of at least 32 for dkLen here.}} + // ^^ + + SCrypt.Generate(bs, bs, 1 << 11, 7, p, 31); + // ^^^^^^^ {{Use a cost factor of at least 2 ^ 12 for N here.}} + // ^ @-1 {{Use a memory factor of at least 8 for r here.}} + // ^^ @-2 {{Use an output length of at least 32 for dkLen here.}} + } + } + """) .Verify(); } diff --git a/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs b/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs index 7c1e50a05d8..4395841f40d 100644 --- a/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs +++ b/analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs @@ -71,6 +71,8 @@ public static class NuGetMetadataReference public static References MicrosoftAspNetCoreRouting(string packageVersion) => Create("Microsoft.AspNetCore.Routing", packageVersion); public static References MicrosoftAspNetCoreMvcRazorRuntime(string packageVersion = "2.2.0") => Create("Microsoft.AspNetCore.Razor.Runtime", packageVersion); public static References MicrosoftAspNetCoreRoutingAbstractions(string packageVersion) => Create("Microsoft.AspNetCore.Routing.Abstractions", packageVersion); + // There is no package version of Microsoft.AspNet.Identity that is NOT a pre-release. + public static References MicrosoftAspNetIdentity(string packageVersion = "3.0.0-rc1-final") => Create("Microsoft.AspNet.Identity", packageVersion); public static References MicrosoftAspNetMvc(string packageVersion) => Create("Microsoft.AspNet.Mvc", packageVersion); public static References MicrosoftAspNetCoreAppRef(string packageVersion) => Create("Microsoft.AspNetCore.App.Ref", packageVersion); public static References MicrosoftAspNetSignalRCore(string packageVersion = "2.4.1") => Create("Microsoft.AspNet.SignalR.Core", packageVersion);