Skip to content

Commit

Permalink
Create rule S5344: Part 2
Browse files Browse the repository at this point in the history
  • Loading branch information
@gregory-paidis-sonarsource
gregory-paidis-sonarsource committed May 16, 2024
1 parent e98c967 commit d10a789
Show file tree
Hide file tree
Showing 4 changed files with 100 additions and 11 deletions.
41 changes: 34 additions & 7 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/PasswordsShouldBeStoredCorrectly.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,28 @@ 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(
propertyTracker,
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")),
propertyTracker.MatchProperty(new MemberDescriptor(KnownType.Microsoft_AspNetCore_Identity_PasswordHasherOptions, "CompatibilityMode")),
x => propertyTracker.AssignedValue(x) is int mode && mode == 0); // PasswordHasherCompatibilityMode.IdentityV2 results to 0

var argumentTracker = CSharpFacade.Instance.Tracker.Argument;
Expand All @@ -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
Expand Down Expand Up @@ -109,16 +120,32 @@ 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<SyntaxKind> tracker) =>
Expand Down
4 changes: 3 additions & 1 deletion analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
64 changes: 61 additions & 3 deletions analyzers/tests/SonarAnalyzer.Test/Rules/PasswordsShouldBeStoredCorrectlyTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,44 @@ 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
Expand Down Expand Up @@ -76,12 +106,12 @@ void Method(int iterations, byte[] bs)
}
}
""")
.AddReferences(MetadataReferenceFacade.SystemSecurityCryptography)
.Verify();

[TestMethod]
public void PasswordsShouldBeStoredCorrectly_CS_BouncyCastle_Generate() =>
Builder
.AddReferences(NuGetMetadataReference.BouncyCastle())
.AddSnippet("""
using Org.BouncyCastle.Crypto.Generators;

Expand Down Expand Up @@ -118,12 +148,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;
Expand All @@ -144,6 +174,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();
}
2 changes: 2 additions & 0 deletions analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Security.Cryptography;
using static SonarAnalyzer.TestFramework.MetadataReferences.NuGetMetadataFactory;
using References = System.Collections.Generic.IEnumerable<Microsoft.CodeAnalysis.MetadataReference>;

Expand Down Expand Up @@ -71,6 +72,7 @@ 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);
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);
Expand Down

0 comments on commit d10a789

Please sign in to comment.