Skip to content

Commit

Permalink
Create rule S4347: 'SecureRandom' seeds should not be predictable (pa…
Browse files Browse the repository at this point in the history 
…rt 3)
  • Loading branch information
@gregory-paidis-sonarsource
gregory-paidis-sonarsource committed May 24, 2024
1 parent 0876a57 commit 5558a7b
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 19 deletions.
43 changes: 34 additions & 9 deletions .../SonarAnalyzer.CSharp/SymbolicExecution/Roslyn/SecureRandomSeedsShouldNotBePredictable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ protected override ProgramState PreProcessSimple(SymbolicContext context)
}
else if (operation.AsObjectCreation() is { } objectCreation)
{
return ProcessObjectCreation(state, objectCreation);
return ProcessRandomGeneratorCreation(state, objectCreation)
?? ProcessSecureRandomCreation(state, objectCreation)
?? state;
}
else if (operation.AsInvocation() is { } invocation)
{
Expand All @@ -72,10 +74,29 @@ protected override ProgramState PreProcessSimple(SymbolicContext context)

// new VmpcRandomGenerator()
// new DigestRandomGenerator(digest)
private static ProgramState ProcessObjectCreation(ProgramState state, IObjectCreationOperationWrapper objectCreation) =>
private static ProgramState ProcessRandomGeneratorCreation(ProgramState state, IObjectCreationOperationWrapper objectCreation) =>
objectCreation.Type.IsAny(KnownType.Org_BouncyCastle_Crypto_Prng_DigestRandomGenerator, KnownType.Org_BouncyCastle_Crypto_Prng_VmpcRandomGenerator)
? state.SetOperationConstraint(objectCreation, CryptographicSeedConstraint.Predictable)
: state;
: null;

// new SecureRandom(generator)
// new SecureRandom(generator, autoSeedLength)
private static ProgramState ProcessSecureRandomCreation(ProgramState state, IObjectCreationOperationWrapper objectCreation)
{
return objectCreation.Type.Is(KnownType.Org_BouncyCastle_Security_SecureRandom)
&& GeneratorConstraint() is { } constraint
&& HasSmallAutoseed()
? state.SetOperationConstraint(objectCreation, constraint)
: null;

CryptographicSeedConstraint GeneratorConstraint() =>
objectCreation.ArgumentValue("generator") is { } generator
? state[generator].Constraint<CryptographicSeedConstraint>()
: null;

bool HasSmallAutoseed() =>
objectCreation.ArgumentValue("autoSeedLengthInBytes") is null or { ConstantValue.HasValue: true, ConstantValue.Value: < 16 };
}

// SecureRandom.GetInstance("algorithm", false)
private static ProgramState ProcessSecureRandomGetInstance(ProgramState state, IInvocationOperationWrapper invocation) =>
Expand All @@ -94,16 +115,16 @@ private static ProgramState ProcessSeedingMethods(ProgramState state, IInvocatio
&& invocation.Instance is { } instance
// If it is already unpredictable, do nothing.
// Seeding methods do not overwrite the state, but _mix_ it with the new value.
&& state[instance]?.HasConstraint(CryptographicSeedConstraint.Unpredictable) is false
&& invocation.ArgumentValue("seed") is { } argumentValue
&& state[instance]?.HasConstraint(CryptographicSeedConstraint.Predictable) is true
&& SeedValue() is { } seedValue
&& invocation.Instance.TrackedSymbol(state) is { } instanceSymbol)
{
var constraint = CryptographicSeedConstraint.Unpredictable;
if (argumentValue.ConstantValue.HasValue)
if (seedValue.ConstantValue.HasValue)
{
constraint = CryptographicSeedConstraint.Predictable;
}
else if (state[argumentValue]?.Constraint<CryptographicSeedConstraint>() is { } value)
else if (state[seedValue]?.Constraint<CryptographicSeedConstraint>() is { } value)
{
constraint = value;
}
Expand All @@ -118,6 +139,11 @@ bool IsSetSeed() =>
bool IsAddSeedMaterial() =>
invocation.TargetMethod.Name == "AddSeedMaterial"
&& IsIRandomGenerator(invocation);

IOperation SeedValue() =>
invocation.ArgumentValue("seed")
?? invocation.ArgumentValue("inSeed") // DigestRandomGenerator renamed the interface arguments
?? invocation.ArgumentValue("rSeed"); // Same as above
}

// secureRandom.NextXXX()
Expand Down Expand Up @@ -145,6 +171,5 @@ private static bool IsSecureRandom(IInvocationOperationWrapper invocation) =>
invocation.TargetMethod.ContainingType.Is(KnownType.Org_BouncyCastle_Security_SecureRandom);

private static bool IsIRandomGenerator(IInvocationOperationWrapper invocation) =>
invocation.Instance is { } instance
&& instance.Type.Is(KnownType.Org_BouncyCastle_Crypto_Prng_IRandomGenerator);
invocation.TargetMethod.ContainingType.DerivesOrImplements(KnownType.Org_BouncyCastle_Crypto_Prng_IRandomGenerator);
}
65 changes: 55 additions & 10 deletions analyzers/tests/SonarAnalyzer.Test/Rules/SecureRandomSeedsShouldNotBePredictableTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class SecureRandomSeedsShouldNotBePredictableTest
private readonly VerifierBuilder builder = new VerifierBuilder()
.AddAnalyzer(() => new CS.SymbolicExecutionRunner(AnalyzerConfiguration.AlwaysEnabled))
.WithOnlyDiagnostics(ChecksCS.SecureRandomSeedsShouldNotBePredictable.S4347)
.AddReferences(NuGetMetadataReference.BouncyCastle());
.AddReferences(NuGetMetadataReference.BouncyCastleCryptography());

[TestMethod]
public void SecureRandomSeedsShouldNotBePredictable_CS() =>
Expand Down Expand Up @@ -152,10 +152,56 @@ void Method(byte b)
""")
.Verify();

[DataRow("DigestRandomGenerator(digest)")]
[DataRow("VmpcRandomGenerator()")]
[DataRow("")]
[DataRow("null")]
[DataRow("new VmpcRandomGenerator()")]
[DataRow("new VmpcRandomGenerator(), 5")]
[DataRow("new VmpcRandomGenerator(), 20")]
[DataRow("new DigestRandomGenerator(null)")]
[DataRow("new DigestRandomGenerator(null), 5")]
[DataRow("new DigestRandomGenerator(null), 20")]
[DataTestMethod]
public void SecureRandomSeedsShouldNotBePredictable_CS_IRandomGenerator(string generator) =>
public void SecureRandomSeedsShouldNotBePredictable_CS_SecureRandom_Inheritance(string arguments) =>
builder
.AddSnippet($$$"""
using Org.BouncyCastle.Crypto.Prng;
using Org.BouncyCastle.Security;

class Testcases
{
const int SEED = 42;

void Method(int seed)
{
SecureRandom.GetInstance("", false).Next(); // Noncompliant, baseline

SecureRandom sr = new MySecureRandom({{{arguments}}});
sr.Next(); // Compliant

sr.SetSeed(SEED);
sr.Next(); // Compliant

sr.SetSeed(seed);
sr.Next(); // Compliant
}
}

class MySecureRandom : SecureRandom
{
public MySecureRandom() { }
public MySecureRandom(IRandomGenerator generator) { }
public MySecureRandom(IRandomGenerator generator, int autoSeedLengthInBytes) { }
}
""")
.Verify();


[DataRow("DigestRandomGenerator(digest)", "DigestRandomGenerator")]
[DataRow("DigestRandomGenerator(digest)", "IRandomGenerator")]
[DataRow("VmpcRandomGenerator()", "VmpcRandomGenerator")]
[DataRow("VmpcRandomGenerator()", "IRandomGenerator")]
[DataTestMethod]
public void SecureRandomSeedsShouldNotBePredictable_CS_IRandomGenerator(string generator, string type) =>
builder
.AddSnippet($$$"""
using System.Text;
Expand All @@ -169,7 +215,7 @@ class Testcases

void Method(byte[] bs, Sha256Digest digest, long seed)
{
IRandomGenerator rng = new {{{generator}}};
{{{type}}} rng = new {{{generator}}};
rng.NextBytes(bs); // Noncompliant {{Set an unpredictable seed before generating random values.}}
// ^^^^^^^^^^^^^^^^^

Expand Down Expand Up @@ -208,14 +254,14 @@ public void SecureRandomSeedsShouldNotBePredictable_CS_IRandomGenerator_Inherita
builder
.AddSnippet($$$"""
using System;
using Org.BouncyCastle.Security;
using Org.BouncyCastle.Crypto.Prng;
using Org.BouncyCastle.Crypto.Digests;

class Testcases
{
void IRandomGenerator(byte[] bs)
void Method(byte[] bs)
{
new VmpcRandomGenerator().NextBytes(bs); // Noncompliant, baseline

var notRelevant = new MyGen();
notRelevant.NextBytes(bs); // Compliant, we only track "Digest" and "Vmpc".
}
Expand All @@ -230,8 +276,7 @@ public void NextBytes(byte[] bytes) { }
public void NextBytes(byte[] bytes, int start, int len) { }
public void NextBytes(Span<byte> bytes) { }
}

""")
.VerifyNoIssues();
.Verify();
#endif
}
93 changes: 93 additions & 0 deletions analyzers/tests/SonarAnalyzer.Test/TestCases/SecureRandomSeedsShouldNotBePredictable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Linq;
using System.Text;
using Org.BouncyCastle.Security;
using Org.BouncyCastle.Crypto.Prng;
using Org.BouncyCastle.Crypto.Digests;

class Testcases
{
Expand Down Expand Up @@ -267,4 +269,95 @@ void SecureRandom_Arrays(byte b, char[] whoKnows)
sr.SetSeed(xs.Cast<byte>().ToArray());
sr.Next(); // Compliant FN, array is predictable
}

void FromArguments(SecureRandom sr, VmpcRandomGenerator rng1, DigestRandomGenerator rng2, int seed, byte[] bs)
{
sr.Next(); // Compliant
sr.SetSeed(SEED);
sr.Next(); // Compliant
sr.SetSeed(seed);
sr.Next(); // Compliant

rng1.NextBytes(bs); // Compliant
rng1.AddSeedMaterial(SEED);
rng1.NextBytes(bs); // Compliant

rng2.NextBytes(bs); // Compliant
rng2.AddSeedMaterial(seed);
rng2.NextBytes(bs); // Compliant

var sr2 = new SecureRandom(rng1);
sr2.Next(); // Compliant

sr2 = new SecureRandom(rng1, 5);
sr2.Next(); // Compliant

sr2 = new SecureRandom(rng1, 20);
sr2.Next(); // Compliant

sr2 = new SecureRandom(rng2);
sr2.Next(); // Compliant

sr2 = new SecureRandom(rng2, 5);
sr2.Next(); // Compliant

sr2 = new SecureRandom(rng2, 20);
sr2.Next(); // Compliant
}

void RandomGenerator_Inside_SecureRandom(byte[] bs, string s)
{
var notHardcoded = Encoding.UTF8.GetBytes(s);
var hardcoded = Convert.FromBase64String("exploding whale");

var generator = new DigestRandomGenerator(new Sha256Digest());
var rng = new SecureRandom(generator);
rng.NextBytes(bs); // Noncompliant

generator = new DigestRandomGenerator(new Sha256Digest());
rng = new SecureRandom(generator, 11);
rng.NextBytes(bs); // Noncompliant

generator = new DigestRandomGenerator(new Sha256Digest());
generator.AddSeedMaterial(hardcoded);
rng = new SecureRandom(generator);
rng.NextBytes(bs); // Noncompliant

generator = new DigestRandomGenerator(new Sha256Digest());
rng = new SecureRandom(generator);
rng.SetSeed(hardcoded);
rng.NextBytes(bs); // Noncompliant

generator = new DigestRandomGenerator(new Sha256Digest());
generator.AddSeedMaterial(hardcoded);
rng = new SecureRandom(generator);
rng.SetSeed(hardcoded);
rng.NextBytes(bs); // Noncompliant

generator = new DigestRandomGenerator(new Sha256Digest());
rng = new SecureRandom(generator, 16);
rng.NextBytes(bs); // Compliant

generator = new DigestRandomGenerator(new Sha256Digest());
generator.AddSeedMaterial(notHardcoded);
rng = new SecureRandom(generator);
rng.SetSeed(hardcoded);
rng.NextBytes(bs); // Compliant

generator = new DigestRandomGenerator(new Sha256Digest());
generator.AddSeedMaterial(hardcoded);
rng = new SecureRandom(generator);
rng.SetSeed(notHardcoded);
rng.NextBytes(bs); // Compliant

generator = new DigestRandomGenerator(new Sha256Digest());
rng = new SecureRandom(generator);
generator.AddSeedMaterial(notHardcoded);
rng.Next(); // Noncompliant FP, we cannot infer that "rng" is safe after the constructor

generator = new DigestRandomGenerator(new Sha256Digest());
rng = new SecureRandom(generator);
rng.SetSeed(notHardcoded);
generator.NextBytes(bs); // Noncompliant FP, we cannot infer that "generator" is safe after the constructor
}
}
1 change: 1 addition & 0 deletions analyzers/tests/SonarAnalyzer.TestFramework/MetadataReferences/NuGetMetadataReference.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public static class NuGetMetadataReference
public static References AzureStorageFilesShares(string packageVersion = Constants.NuGetLatestVersion) => Create("Azure.Storage.Files.Shares", packageVersion);
public static References AzureStorageQueues(string packageVersion = Constants.NuGetLatestVersion) => Create("Azure.Storage.Queues", packageVersion);
public static References BouncyCastle(string packageVersion = "1.8.5") => Create("BouncyCastle", packageVersion);
public static References BouncyCastleCryptography(string packageVersion = Constants.NuGetLatestVersion) => Create("BouncyCastle.Cryptography", packageVersion);
public static References CastleCore(string packageVersion = "5.1.1") => Create("Castle.Core", packageVersion);
public static References Dapper(string packageVersion = "1.50.5") => Create("Dapper", packageVersion);
public static References CommonLoggingCore(string packageVersion = Constants.NuGetLatestVersion) => Create("Common.Logging.Core", packageVersion);
Expand Down

0 comments on commit 5558a7b

Please sign in to comment.