Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create rule S4347: Secure random number generators must not output predictable values #8992

Closed
jamie-anderson-sonarsource opened this issue Mar 28, 2024 · 0 comments
Assignees
Labels
Area: CFG/SE CFG and SE related issues. Area: Security Related to Vulnerability and Security Hotspot rules Type: New Rule Implementation for a rule that HAS been specified.
Milestone

Comments

@jamie-anderson-sonarsource
Copy link

jamie-anderson-sonarsource commented Mar 28, 2024

Why

As part of MMF-3716, we want to close the gap between C# and other languages regarding cryptography-related rules support. S4347 is one of the rules that is not currently supported by this analyzer.

What

S4347 aims to detect when a secure random number generator is used in a way where the output can become predictable. This is usually because it is not initialized correctly, or it is provided with a seed value that results in predictable output.

Detection Logic

.NET Framework and .NET Core

The random generators inside System.Security.Cryptography have no way to set a seed and are therefore always unpredictable.

BouncyCastle.NET

Most of BouncyCastle's random number generators will create unpredictable output by default. There are only a small number of scenarios (see below) where a predictable random generator can be created. Each of these scenarios requires an unpredictable seed to be set before the random generator can create unpredictable output.

Ideally, these random generators would be seeded with at least 16 bytes of unpredictable material. This prevents a brute-force attack from guessing which seeds were used to create a set of given random values.

However, given the low expected impact of this rule (see below), the initial implementation will consider any byte[], Span<byte> or ReadOnlySpan<byte> to be unpredictable as long as it wasn't created from static data.

The following methods are examples of how a seed can be created from static data:

  • new byte[] { /* static values */ }
  • byte[] System.Text.Encoding.GetBytes(string) where the string is static
  • byte[] System.Convert.FromBase64String(string) where the string is static
  • byte[] Guid.ToByteArray() except where the Guid was created by Guid.NewGuid()

Scenario 1: SecureRandom

S4347 should be raised when the following conditions are met.

  1. A new SecureRandom instance is created by calling Org.BouncyCastle.Security.SecureRandom.GetInstance(string algorithm, bool autoSeed) where autoSeed = false
  2. There are no calls to .SetSeed(...) where the parameter is considered unpredictable (see above).
  3. A random value is generated using one of the following methods:
    • .NextBytes(...)
    • .Next(...)
    • .NextInt()
    • .NextLong()
    • .NextDouble()
using Org.BouncyCastle.Security;

var randomBytes = new byte[8];

var rng1 = SecureRandom.GetInstance("SHA256PRNG", true);
rng1.NextBytes(randomBytes); // compliant

var rng2 = SecureRandom.GetInstance("SHA256PRNG", false);
rng2.NextBytes(randomBytes); // noncompliant

var rng3 = SecureRandom.GetInstance("SHA256PRNG", false);
rng3.SetSeed(Encoding.UTF8.GetBytes("predictable seed")); // predictable seed material
rng3.NextBytes(randomBytes); // noncompliant

var rng4 = SecureRandom.GetInstance("SHA256PRNG", false);
rng4.SetSeed(Guid.NewGuid().ToBytes()); // unpredictable seed material
rng4.NextBytes(randomBytes); // compliant

Scenario 2: IRandomGenerator

S4347 should be raised when the following conditions are met.

  1. An IRandomGenerator instance is created using one of the following constructors:
    • new Org.BouncyCastle.Crypto.Prng.DigestRandomGenerator(Org.BouncyCastle.Crypto.IDigest digest)
    • new Org.BouncyCastle.Crypto.Prng.VmpcRandomGenerator()
  2. There are no calls to .AddSeedMaterial(...) where the parameter is considered unpredictable (see above).
  3. A random value is generated using the one of the .NextBytes(...) methods.
using Org.BouncyCastle.Crypto.Digests;
using Org.BouncyCastle.Crypto.Prng;
using Org.BouncyCastle.Security;

var randomBytes = new byte[8];

var rng1 = new DigestRandomGenerator(new Sha256Digest());
rng1.NextBytes(randomBytes); // noncompliant

var rng2 = new DigestRandomGenerator(new Sha256Digest());
rng2.AddSeedMaterial(Encoding.UTF8.GetBytes("seed value")); // predictable seed material
rng2.NextBytes(randomBytes); // noncompliant

var rng3 = new DigestRandomGenerator(new Sha256Digest());
rng3.AddSeedMaterial(Guid.NewGuid().ToBytes()); // unpredictable seed material
rng3.NextBytes(randomBytes); // compliant

var rng4 = new VmpcRandomGenerator();
rng4.NextBytes(randomBytes); // noncompliant

var rng5 = new VmpcRandomGenerator();
rng5.AddSeedMaterial(SecureRandom.GenerateSeed(16)); // unpredictable seed material
rng5.NextBytes(randomBytes); // compliant

Scenario 3: IRandomGenerator wrapped by SecureRandom

S4347 should be raised when the following conditions are met.

  1. An IRandomGenerator instance (generator) is created using one of the following constructors:
    • new Org.BouncyCastle.Crypto.Prng.DigestRandomGenerator(Org.BouncyCastle.Crypto.IDigest digest)
    • new Org.BouncyCastle.Crypto.Prng.VmpcRandomGenerator()
  2. A new SecureRandom instance (rng) is created to wrap generator, using one of the following constructors:
    • new Org.BouncyCastle.Security.SecureRandom(generator)
    • new Org.BouncyCastle.Security.SecureRandom(generator, autoSeedLength) where autoSeedLength is less than 16.
  3. There are no calls to add unpredictable seed material to generator using either of the following methods:
    • generator.AddSeedMaterial(...)
    • rng.SetSeed(...)
  4. A random value is generated using one of the following methods:
    • generator.NextBytes(...)
    • rng.NextBytes(...)
    • rng.Next(...)
    • rng.NextInt()
    • rng.NextLong()
    • rng.NextDouble()
using Org.BouncyCastle.Crypto.Digests;
using Org.BouncyCastle.Crypto.Prng;
using Org.BouncyCastle.Security;

var randomBytes = new byte[8];

var generator1 = new DigestRandomGenerator(new Sha256Digest());
var rng1 = new SecureRandom(generator1);
rng1.NextBytes(randomBytes); // noncompliant

var generator2 = new DigestRandomGenerator(new Sha256Digest());
var rng2 = new SecureRandom(generator2, 16);
rng2.NextBytes(randomBytes); // compliant

var generator3 = new DigestRandomGenerator(new Sha256Digest());
generator3.AddSeedMaterial(Guid.NewGuid().ToBytes()); // unpredictable seed material
var rng3 = new SecureRandom(generator3);
rng3.NextBytes(randomBytes); // compliant

var generator4 = new DigestRandomGenerator(new Sha256Digest());
var rng4 = new SecureRandom(generator4);
rng4.SetSeed(Guid.NewGuid().ToBytes()); // unpredictable seed material
rng4.NextBytes(randomBytes); // compliant

var generator5 = new DigestRandomGenerator(new Sha256Digest());
generator5.AddSeedMaterial(Encoding.UTF8.GetBytes("seed value")); // predictable seed material
var rng5 = new SecureRandom(generator5);
rng5.SetSeed(Encoding.UTF8.GetBytes("seed value")); // predictable seed material
rng5.NextBytes(randomBytes); // noncompliant

var generator6 = new DigestRandomGenerator(new Sha256Digest());
var rng6 = new SecureRandom(generator5);
rng6.SetSeed(Guid.NewGuid().ToBytes()); // unpredictable seed material
generator6.NextBytes(randomBytes); // compliant

Highlighting and Error Message

The primary highlight should be the RNG instance plus the function name that generates the random value, e.g. rng1.NextBytes.

A secondary highlight should be added to the location where the RNG was created, either via SecureRandom.GetInstance or new ___RandomGenerator.

For scenario 3, another secondary highlight should be added to the location where new SecureRandom(...) is called.

Use the following error message:

Set an unpredictable seed before generating random values.

Expected Impact

SourceGraph does not reveal any public code where noncompliant patterns are in use.

The primary reason for implementing this rule is to ensure coverage for compliance schemes.

Associated RSPEC

When this change is in place, please merge the associated RSPEC content: RSPEC PR #3837

@mary-georgiou-sonarsource mary-georgiou-sonarsource added Type: New Rule Implementation for a rule that HAS been specified. Area: Security Related to Vulnerability and Security Hotspot rules labels Mar 28, 2024
@jamie-anderson-sonarsource jamie-anderson-sonarsource changed the title Create rule S4347: Secure random generators must not output predictable values Create rule S4347: Secure random number generators must not output predictable values Mar 28, 2024
@gregory-paidis-sonarsource gregory-paidis-sonarsource added the Area: CFG/SE CFG and SE related issues. label May 13, 2024
@zsolt-kolbay-sonarsource zsolt-kolbay-sonarsource added this to the 9.26 milestone May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: CFG/SE CFG and SE related issues. Area: Security Related to Vulnerability and Security Hotspot rules Type: New Rule Implementation for a rule that HAS been specified.
Projects
None yet
Development

No branches or pull requests

5 participants