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

Insecure Random Number Generation #3431

Closed
dusmart opened this issue Jul 19, 2024 · 4 comments
Closed

Insecure Random Number Generation #3431

dusmart opened this issue Jul 19, 2024 · 4 comments
Labels
Discussion Initial issue state - proposed but not yet accepted

Comments

@dusmart
Copy link

dusmart commented Jul 19, 2024

Security Issue: Insecure Random Number Generation in NEO Blockchain

Description

  • Version: v3.7.5
  • Module: Plugins/DBFTPlugin

private static ulong GetNonce()
{
Random _random = new();
Span<byte> buffer = stackalloc byte[8];
_random.NextBytes(buffer);
return BinaryPrimitives.ReadUInt64LittleEndian(buffer);
}

The issue lies in the use of the Random class for generating random numbers, specifically the instantiation Random _random = new();. It is well-known that the Random constructor uses a default seed value. Referencing dotnet's manual here. Usually the seed is derived from system clock and some other factors like another PRNG. And the timestamp of the consensus is exposed in the block header which makes it more easier for guessing the seed.

Impact

This predictability poses a significant security risk. By analyzing a large set of published block data, there exists possibility to infer the random seed sequence of a consensus node. With this information, one could predict some blocks' nonce values of future blocks. Therefore, one can manipulate a transactions's nonce so that the value got from System.Runtime.GetRandom is preferable.

The block nonce are the crucial random source for various on-chain applications. Applications relying on these random source include:

  • Assigning attributes to NFTs
  • On-chain gambling games, such as FTW

Suggested Fix

To mitigate this issue, I recommend using a cryptographically secure random number generator, such as RNGCryptoServiceProvider in C#. This will ensure that the random numbers used in block generation are not predictable and significantly enhance the security of the blockchain.

Possible Example:

using (RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider())
{
    byte[] randomNumber = new byte[8];
    rng.GetBytes(randomNumber);
    int value = BitConverter.ToInt32(randomNumber, 0);
}

Conclusion

The current method of random number generation in the NEO blockchain is insecure and can be exploited to predict block nonce values. This poses a serious risk to the integrity of on-chain applications. Implementing a cryptographically secure random number generator will address this vulnerability.

@dusmart dusmart added the Discussion Initial issue state - proposed but not yet accepted label Jul 19, 2024
@dusmart
Copy link
Author

dusmart commented Jul 19, 2024

@vncoelho
Copy link
Member

Agreed

@cschuchardt88 cschuchardt88 self-assigned this Jul 19, 2024
@cschuchardt88
Copy link
Member

cschuchardt88 commented Jul 19, 2024

The random data should be hashed

Example:

static ulong GetNonce()
{
    Span<byte> seed = stackalloc byte[8];
    Span<byte> source = stackalloc byte[8];
    RandomNumberGenerator.Fill(seed);
    RandomNumberGenerator.Fill(source);
    var hash = HMACSHA384.HashData(seed, source);
    var start = Math.Abs(new Random(Random.Shared.Next()).Next(buffer.Length - 8));
    return BinaryPrimitives.ReadUInt64LittleEndian(hash[start..]);
}

Outputs

Round 1 : 13932607928842274499
Round 2 : 1486748283979591455
Round 3 : 3436167571153346206
Round 4 : 17404178586397639120
Round 5 : 5702540448270665803
Round 6 : 11570980513260468740
Round 7 : 3998776913995718958
Round 8 : 11671218581804551298
Round 9 : 3537788841882644554
Round 10: 6683530947903016615

@cschuchardt88 cschuchardt88 removed their assignment Jul 19, 2024
@dusmart
Copy link
Author

dusmart commented Jul 23, 2024

Closed by #3432

@dusmart dusmart closed this as completed Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

No branches or pull requests

3 participants