diff --git a/analyzers/rspec/cs/S4347.html b/analyzers/rspec/cs/S4347.html new file mode 100644 index 00000000000..853690d0de6 --- /dev/null +++ b/analyzers/rspec/cs/S4347.html @@ -0,0 +1,135 @@ +

When using SecureRandom, it is important not to use predictable seeds. This class is used to generate cryptographically strong random +numbers. Using a predictable seed will make its output predictable as well, which counteracts the use case of SecureRandom.

+

Why is this an issue?

+

java.security.SecureRandom is often used to generate random values for cryptographic algorithms. When a random number generator is +used for cryptographic purposes, the generated numbers must be as random and unpredictable as possible. When SecureRandom is improperly +seeded with a constant or a predictable value, its output will also be predictable.

+

This can have severe security implications for cryptographic operations that rely on the randomness of the generated numbers. By using a +predictable seed, an attacker can potentially guess or deduce the generated numbers, compromising the security of whatever cryptographic algorithm +relies on SecureRandom.

+

What is the potential impact?

+

It is crucial to understand that the strength of cryptographic algorithms heavily relies on the quality of the random numbers used. By improperly +seeding the SecureRandom class, we introduce a significant weakness that can be exploited by attackers.

+

Insecure cryptographic keys

+

One of the primary use cases for the SecureRandom class is generating cryptographic keys. If an attacker can predict the seed used to +initialize the SecureRandom instance, they may be able to derive the same keys. Depending on the use case, this can lead to multiple severe outcomes, +such as:

+ +

Session hijacking and man-in-the-middle attack

+

Another scenario where this vulnerability can be exploited is in the generation of session tokens or nonces for secure communication protocols. If +an attacker can predict the seed used to generate these tokens, they can impersonate legitimate users or intercept sensitive information.

+

How to fix it in BouncyCastle

+

BouncyCastle provides several random number generators implementations. Most of these will automatically create unpredictable output.

+

The remaining random number generators require seeding with an unpredictable value before they will produce unpredictable outputs. These should be +seeded with at least 16 bytes of random data to ensure unpredictable output and that the random seed cannot be guessed using a brute-force attack.

+

Code examples

+

Noncompliant code example

+

SecureRandom instances created with GetInstance() are seeded by default. Disabling seeding results in predictable +output.

+
+using Org.BouncyCastle.Security;
+
+byte[] random = new byte[8];
+
+SecureRandom sr = SecureRandom.GetInstance("SHA256PRNG", false);
+sr.NextBytes(random); // Noncompliant
+
+

DigestRandomGenerator and VmpcRandomGenerator instances require seeding before use. Predictable seed values will result +in predictable outputs.

+
+using Org.BouncyCastle.Crypto.Digest;
+using Org.BouncyCastle.Crypto.Prng;
+
+byte[] random = new byte[8];
+
+IRandomGenerator digest = new DigestRandomGenerator(new Sha256Digest());
+digest.AddSeedMaterial(Encoding.UTF8.GetBytes("predictable seed value"));
+digest.NextBytes(random); // Noncompliant
+
+IRandomGenerator vmpc = new VmpcRandomGenerator();
+vmpc.AddSeedMaterial(Convert.FromBase64String("2hq9pkyqLQJkrYTrEv1eNw=="));
+vmpc.NextBytes(random); // Noncompliant
+
+

When a SecureRandom is created using an unseeded DigestRandomGenerator and VmpcRandomGenerator instance, the +SecureRandom will create predictable outputs.

+
+using Org.BouncyCastle.Crypto.Digest;
+using Org.BouncyCastle.Crypto.Prng;
+using Org.BouncyCastle.Security;
+
+byte[] random = new byte[8];
+
+IRandomGenerator digest = new DigestRandomGenerator(new Sha256Digest());
+SecureRandom sr = new SecureRandom(digest);
+sr.NextBytes(random); // Noncompliant
+
+

Compliant solution

+

Allow SecureRandom.GetInstance() to automatically seed new SecureRandom instances.

+
+using Org.BouncyCastle.Security;
+
+byte[] random = new byte[8];
+
+SecureRandom sr = SecureRandom.GetInstance("SHA256PRNG");
+sr.NextBytes(random);
+
+

Use unpredictable values to seed DigestRandomGenerator and VmpcRandomGenerator instances. The +SecureRandom.GenerateSeed() method is designed for this purpose.

+
+using Org.BouncyCastle.Crypto.Digest;
+using Org.BouncyCastle.Crypto.Prng;
+using Org.BouncyCastle.Security;
+
+byte[] random = new byte[8];
+
+IRandomGenerator digest = new DigestRandomGenerator(new Sha256Digest());
+digest.AddSeedMaterial(SecureRandom.GenerateSeed(16));
+digest.NextBytes(random);
+
+IRandomGenerator vmpc = new VmpcRandomGenerator();
+vmpc.AddSeedMaterial(SecureRandom.GenerateSeed(16));
+vmpc.NextBytes(random);
+
+

An overload of the SecureRandom constructor will automatically seed the underlying IRandomGenerator with an unpredictable +value.

+
+using Org.BouncyCastle.Crypto.Digest;
+using Org.BouncyCastle.Crypto.Prng;
+using Org.BouncyCastle.Security;
+
+byte[] random = new byte[8];
+
+IRandomGenerator digest = new DigestRandomGenerator(new Sha256Digest());
+SecureRandom sr = new SecureRandom(digest, 16);
+sr.NextBytes(random);
+
+

Resources

+

Documentation

+ +

Standards

+ +

Implementation Specification

+

(visible only on this page)

+

Message

+

Change this seed value to something unpredictable, or remove the seed.

+

Highlighting

+

The call to SecureRandom.setSeed() or the SecureRandom(byte[]) constructor call

+ diff --git a/analyzers/rspec/cs/S4347.json b/analyzers/rspec/cs/S4347.json new file mode 100644 index 00000000000..1ec65b7e40a --- /dev/null +++ b/analyzers/rspec/cs/S4347.json @@ -0,0 +1,43 @@ +{ + "title": "\"SecureRandom\" seeds should not be predictable", + "type": "VULNERABILITY", + "code": { + "impacts": { + "SECURITY": "HIGH" + }, + "attribute": "LOGICAL" + }, + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "2min" + }, + "tags": [ + "cwe", + "pitfall" + ], + "defaultSeverity": "Critical", + "ruleSpecification": "RSPEC-4347", + "sqKey": "S4347", + "scope": "Main", + "securityStandards": { + "CWE": [ + 330, + 332, + 336, + 337 + ], + "OWASP": [ + "A6" + ], + "OWASP Top 10 2021": [ + "A2" + ], + "ASVS 4.0": [ + "2.3.1", + "2.6.2", + "2.9.2" + ] + }, + "quickfix": "unknown" +} diff --git a/analyzers/rspec/cs/Sonar_way_profile.json b/analyzers/rspec/cs/Sonar_way_profile.json index 124867bd2c3..47734b1b31f 100644 --- a/analyzers/rspec/cs/Sonar_way_profile.json +++ b/analyzers/rspec/cs/Sonar_way_profile.json @@ -246,6 +246,7 @@ "S4260", "S4275", "S4277", + "S4347", "S4423", "S4426", "S4428", diff --git a/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs b/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs index 22b807d26d8..7f375fa0213 100644 --- a/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs +++ b/analyzers/src/SonarAnalyzer.CSharp/Rules/SymbolicExecution/SymbolicExecutionRunner.cs @@ -34,10 +34,6 @@ namespace SonarAnalyzer.Rules.CSharp; [DiagnosticAnalyzer(LanguageNames.CSharp)] public class SymbolicExecutionRunner : SymbolicExecutionRunnerBase { - public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { } - - internal /* for testing */ SymbolicExecutionRunner(IAnalyzerConfiguration configuration) : base(configuration) { } - protected override ImmutableDictionary AllRules { get; } = ImmutableDictionary.Empty .Add(InvalidCastToInterface.S1944, CreateFactory()) // This old SE rule is part of S3655. .Add(HashesShouldHaveUnpredictableSalt.S2053, CreateFactory()) @@ -51,11 +47,14 @@ public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { } .Add(CalculationsShouldNotOverflow.S3949, CreateFactory()) .Add(ObjectsShouldNotBeDisposedMoreThanOnce.S3966, CreateFactory()) .Add(EmptyCollectionsShouldNotBeEnumerated.S4158, CreateFactory()) - .Add(RestrictDeserializedTypes.S5773, CreateFactory()); + .Add(RestrictDeserializedTypes.S5773, CreateFactory()) + .Add(SecureRandomSeedsShouldNotBePredictable.S4347, CreateFactory()); + protected override SyntaxClassifierBase SyntaxClassifier => CSharpSyntaxClassifier.Instance; public override ImmutableArray SupportedDiagnostics => base.SupportedDiagnostics.ToImmutableArray(); - protected override SyntaxClassifierBase SyntaxClassifier => CSharpSyntaxClassifier.Instance; + public SymbolicExecutionRunner() : this(AnalyzerConfiguration.AlwaysEnabled) { } + internal /* for testing */ SymbolicExecutionRunner(IAnalyzerConfiguration configuration) : base(configuration) { } protected override void Initialize(SonarAnalysisContext context) { diff --git a/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs new file mode 100644 index 00000000000..d9a82cdda64 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs @@ -0,0 +1,156 @@ +/* + * 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 System.Text; +using SonarAnalyzer.Common.Walkers; +using SonarAnalyzer.SymbolicExecution.Constraints; + +namespace SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks.CSharp; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class SecureRandomSeedsShouldNotBePredictable : SymbolicRuleCheck +{ + private const string DiagnosticId = "S4347"; + private const string MessageFormat = "Set an unpredictable seed before generating random values."; + private static readonly ImmutableHashSet SecureRandomNextMethods = ImmutableHashSet.Create( + "Next", + "NextInt", + "NextLong", + "NextDouble", + "NextBytes"); + + public static readonly DiagnosticDescriptor S4347 = DescriptorFactory.Create(DiagnosticId, MessageFormat); + protected override DiagnosticDescriptor Rule => S4347; + + public override bool ShouldExecute() + { + var walker = new Walker(); + walker.SafeVisit(Node); + return walker.Result; + } + + protected override ProgramState PreProcessSimple(SymbolicContext context) + { + var state = base.PreProcessSimple(context); + var operation = context.Operation.Instance; + + if (operation.AsArrayCreation() is { } arrayCreation) + { + return ProcessArrayCreation(state, arrayCreation); + } + else if (operation.AsInvocation() is { } invocation) + { + return ProcessSecureRandomGetInstance(state, invocation) + ?? ProcessStringToBytes(state, invocation) + ?? ProcessSecureRandomSetSeed(state, invocation) + ?? ProcessSecureRandomNext(state, invocation); + } + return state; + } + + // TODO: Handle array access or simple assignment when someone messes with the byte array + // TODO: Handle Guids + // TODO: Handle char array overloads of Encoding+Convert (Optional) + private static ProgramState ProcessArrayCreation(ProgramState state, IArrayCreationOperationWrapper arrayCreation) + { + if (arrayCreation.Type.IsAny(KnownType.System_Byte_Array, KnownType.System_Char_Array)) + { + var isConstant = arrayCreation.Initializer.WrappedOperation is null || arrayCreation.Initializer.ElementValues.All(x => x.ConstantValue.HasValue); + return state.SetOperationConstraint( + arrayCreation, + isConstant ? CryptographicSeedConstraint.Predictable : CryptographicSeedConstraint.Unpredictable); + } + return state; + } + + private static ProgramState ProcessSecureRandomGetInstance(ProgramState state, IInvocationOperationWrapper invocation) => + invocation.TargetMethod.Name == "GetInstance" + && invocation.TargetMethod.ContainingType.DerivesFrom(KnownType.Org_BouncyCastle_Security_SecureRandom) + && invocation.Arguments.Length == 2 + && invocation.ArgumentValue("autoSeed") is { ConstantValue: { HasValue: true, Value: false } } + ? state.SetOperationConstraint(invocation, CryptographicSeedConstraint.Predictable) + : null; + + private ProgramState ProcessStringToBytes(ProgramState state, IInvocationOperationWrapper invocation) + { + return (IsEncodingGetBytes() || IsConvertFromBase64String()) + && invocation.ArgumentValue("s") is { ConstantValue.HasValue: true } + ? state.SetOperationConstraint(invocation, CryptographicSeedConstraint.Predictable) + : null; + + bool IsEncodingGetBytes() => + invocation.TargetMethod.Name == nameof(Encoding.UTF8.GetBytes) + && invocation.TargetMethod.ContainingType.DerivesFrom(KnownType.System_Text_Encoding); + + bool IsConvertFromBase64String() => + invocation.TargetMethod.Name == nameof(Convert.FromBase64String) + && invocation.TargetMethod.ContainingType.Is(KnownType.System_Convert); + } + + private static ProgramState ProcessSecureRandomSetSeed(ProgramState state, IInvocationOperationWrapper invocation) + { + if (invocation.TargetMethod.Name == "SetSeed" + && invocation.TargetMethod.ContainingType.DerivesFrom(KnownType.Org_BouncyCastle_Security_SecureRandom) + && state[invocation.Instance]?.HasConstraint(CryptographicSeedConstraint.Unpredictable) is false // If it is already unpredictable, do nothing + && invocation.ArgumentValue("seed") is { } argumentValue) + { + var constraint = CryptographicSeedConstraint.Unpredictable; + + if (argumentValue.ConstantValue.HasValue) + { + constraint = CryptographicSeedConstraint.Predictable; + } + else if (state[argumentValue]?.Constraint() is { } value) + { + constraint = value; + } + return state.SetSymbolConstraint(invocation.Instance.TrackedSymbol(state), constraint); + } + return null; + } + + private ProgramState ProcessSecureRandomNext(ProgramState state, IInvocationOperationWrapper invocation) + { + if (SecureRandomNextMethods.Contains(invocation.TargetMethod.Name) + && invocation.TargetMethod.ContainingType.DerivesFrom(KnownType.Org_BouncyCastle_Security_SecureRandom) + && state[invocation.Instance]?.HasConstraint(CryptographicSeedConstraint.Predictable) is true) + { + ReportIssue(invocation.WrappedOperation); + } + return state; + } + + private sealed class Walker : SafeCSharpSyntaxWalker + { + public bool Result { get; private set; } + + public override void Visit(SyntaxNode node) + { + if (!Result) + { + base.Visit(node); + } + } + + // TODO: Check if we can make this short-circuit more efficiently + public override void VisitInvocationExpression(InvocationExpressionSyntax node) => + Result = node.NameIs("GetInstance"); + } +} diff --git a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs index 38841ef5399..1eefea92653 100644 --- a/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs +++ b/analyzers/src/SonarAnalyzer.Common/Helpers/KnownType.cs @@ -211,6 +211,7 @@ public sealed partial class KnownType public static readonly KnownType Org_BouncyCastle_Crypto_Generators_DHParametersGenerator = new("Org.BouncyCastle.Crypto.Generators.DHParametersGenerator"); 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_Security_SecureRandom = new("Org.BouncyCastle.Security.SecureRandom"); 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"); @@ -235,6 +236,8 @@ public sealed partial class KnownType public static readonly KnownType System_Byte = new("System.Byte"); public static readonly KnownType System_Byte_Array = new("System.Byte") { IsArray = true }; public static readonly KnownType System_Char = new("System.Char"); + public static readonly KnownType System_Char_Array = new("System.Char") { IsArray = true }; + public static readonly KnownType System_Convert = new("System.Convert"); public static readonly KnownType System_CLSCompliantAttribute = new("System.CLSCompliantAttribute"); public static readonly KnownType System_CodeDom_Compiler_GeneratedCodeAttribute = new("System.CodeDom.Compiler.GeneratedCodeAttribute"); public static readonly KnownType System_Collections_CollectionBase = new("System.Collections.CollectionBase"); diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/ConstraintKind.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/ConstraintKind.cs index 4757aa58c88..ca27f823e9f 100644 --- a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/ConstraintKind.cs +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/ConstraintKind.cs @@ -32,6 +32,8 @@ public enum ConstraintKind CollectionNotEmpty, CryptographicallyStrong, CryptographicallyWeak, + CryptographicallyPredictable, + CryptographicallyUnpredictable, DisposableDisposed, InitializationVectorInitialized, InitializationVectorNotInitialized, diff --git a/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CryptographicSeedConstraint.cs b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CryptographicSeedConstraint.cs new file mode 100644 index 00000000000..71e9d9dcce9 --- /dev/null +++ b/analyzers/src/SonarAnalyzer.Common/SymbolicExecution/Constraints/CryptographicSeedConstraint.cs @@ -0,0 +1,31 @@ +/* + * 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. + */ + +namespace SonarAnalyzer.SymbolicExecution.Constraints; + +internal class CryptographicSeedConstraint : SymbolicConstraint +{ + public static readonly CryptographicSeedConstraint Predictable = new(ConstraintKind.CryptographicallyPredictable); + public static readonly CryptographicSeedConstraint Unpredictable = new(ConstraintKind.CryptographicallyUnpredictable); + + public override SymbolicConstraint Opposite => this == Predictable ? Unpredictable : Predictable; + + private CryptographicSeedConstraint(ConstraintKind kind) : base(kind) { } +} diff --git a/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs b/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs index fe50700def8..a4495cbb262 100644 --- a/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs +++ b/analyzers/tests/SonarAnalyzer.Test/PackagingTests/RuleTypeMappingCS.cs @@ -4271,7 +4271,7 @@ internal static class RuleTypeMappingCS // ["S4344"], // ["S4345"], // ["S4346"], - // ["S4347"], + ["S4347"] = "VULNERABILITY", // ["S4348"], // ["S4349"], // ["S4350"], diff --git a/analyzers/tests/SonarAnalyzer.Test/Rules/SecureRandomSeedsShouldNotBePredictableTest.cs b/analyzers/tests/SonarAnalyzer.Test/Rules/SecureRandomSeedsShouldNotBePredictableTest.cs new file mode 100644 index 00000000000..5dc72076ebc --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/Rules/SecureRandomSeedsShouldNotBePredictableTest.cs @@ -0,0 +1,116 @@ +/* + * 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 System.Text; +using ChecksCS = SonarAnalyzer.SymbolicExecution.Roslyn.RuleChecks.CSharp; +using CS = SonarAnalyzer.Rules.CSharp; + +namespace SonarAnalyzer.Test.Rules; + +[TestClass] +public class SecureRandomSeedsShouldNotBePredictableTest +{ + private readonly VerifierBuilder builder = new VerifierBuilder() + .AddAnalyzer(() => new CS.SymbolicExecutionRunner(AnalyzerConfiguration.AlwaysEnabled)) + .WithOnlyDiagnostics(ChecksCS.SecureRandomSeedsShouldNotBePredictable.S4347) + .AddReferences(NuGetMetadataReference.BouncyCastle()); + + [TestMethod] + public void SecureRandomSeedsShouldNotBePredictable_CS() => + builder + .AddPaths("SecureRandomSeedsShouldNotBePredictable.cs") + .Verify(); + + [DataRow("sr.Next()")] + [DataRow("sr.Next(42)")] + [DataRow("sr.Next(0, 42)")] + [DataRow("sr.NextBytes(new byte[8])")] + [DataRow("sr.NextInt()")] + [DataRow("sr.NextLong()")] + [DataRow("sr.NextDouble()")] + [DataTestMethod] + public void SecureRandomSeedsShouldNotBePredictable_CS_Next_NoSetSeed(string expression) => + builder + .AddSnippet($$$""" + using Org.BouncyCastle.Security; + class Testcases + { + void Method() + { + var sr = SecureRandom.GetInstance("SHA256PRNG", false); + {{{expression}}}; // Noncompliant {{Set an unpredictable seed before generating random values.}} + + sr = SecureRandom.GetInstance("SHA256PRNG", true); + {{{expression}}}; // Compliant, autoSeed is set to true + + sr = SecureRandom.GetInstance("SHA256PRNG"); + {{{expression}}}; // Compliant, autoSeed is implicitly set to true + } + } + """) + .Verify(); + + [DataRow("new byte[42]")] + [DataRow("new byte[] { 1, 2, 3 }")] + [DataRow("new byte[] { (byte)'a', (byte)'b', (byte)'c' }")] + [DataRow("""Encoding.UTF8.GetBytes("exploding whale")""")] + [DataRow("""System.Convert.FromBase64String("exploding whale")""")] + //[DataRow("new Guid().ToByteArray()")] + //[DataRow("new Guid().ToByteArray(true)")] + //[DataRow("new Guid().ToByteArray(false)")] + [DataTestMethod] + public void SecureRandomSeedsShouldNotBePredictable_CS_Next_WithSetSeed(string expression) => + builder + .AddSnippet($$$""" + using System; + using System.Text; + using Org.BouncyCastle.Security; + class Testcases + { + void StartWithUnpredictable(byte[] whoKnows) + { + var sr = SecureRandom.GetInstance("SHA256PRNG", false); + + sr.SetSeed(whoKnows); + sr.Next(); // Compliant, seed is updated with unknown data + + sr.SetSeed({{{expression}}}); + sr.Next(); // Compliant, seed is updated with non-random data, but it (might) have already been random + } + + void StartWithPredictable() + { + var sr = SecureRandom.GetInstance("SHA256PRNG", false); + + var seed = {{{expression}}}; + sr.SetSeed(seed); + sr.Next(); // Noncompliant {{Set an unpredictable seed before generating random values.}} + // ^^^^^^^^^ + + sr.SetSeed(Guid.NewGuid().ToByteArray()); + sr.Next(); // Compliant, seed is updated with random data + + sr.SetSeed({{{expression}}}); + sr.Next(); // Compliant, seed is updated with non-random data, but it is already random + } + } + """) + .Verify(); +} diff --git a/analyzers/tests/SonarAnalyzer.Test/TestCases/SecureRandomSeedsShouldNotBePredictable.cs b/analyzers/tests/SonarAnalyzer.Test/TestCases/SecureRandomSeedsShouldNotBePredictable.cs new file mode 100644 index 00000000000..7e0e25c2af0 --- /dev/null +++ b/analyzers/tests/SonarAnalyzer.Test/TestCases/SecureRandomSeedsShouldNotBePredictable.cs @@ -0,0 +1,24 @@ +using Org.BouncyCastle.Security; +using System; + +class Testcases +{ + const int SEED = 42; + + void GetInstance_SetSeed_Next(byte[] bs, bool condition) + { + SecureRandom sr = SecureRandom.GetInstance("SHA256PRNG", false); + sr.Next(); // Noncompliant {{Set an unpredictable seed before generating random values.}} + // ^^^^^^^^^ + + sr = SecureRandom.GetInstance("SHA256PRNG", false); + sr.SetSeed(SEED); + sr.Next(); // Noncompliant {{Set an unpredictable seed before generating random values.}} + // ^^^^^^^^^ + } + + // TODO: Add tests for conditional if/else/switch + // TODO: Add tests for for/while/goto loops + // TODO: Add tests for try/catch/finally regions + +}