HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Cryptographically Secure Pin Generator using RNGCryptoServiceProvider

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
securerngcryptoserviceprovidergeneratorcryptographicallyusingpin

Problem

In my system I have a SMS authentication. Users input their user-id and get sent an SMS with a 5 digit pin number they input in a website to log in. The pin has a lifetime of a few minutes.

This pin number needs to be fairly secure so hackers can't try to guess the pin.

I would like to get some comments about my approach and code.

Here is my Code:

public class PinGenerator : IPinGenerator
{
    public string GetPin()
    {
        var rand = GetRandomNumber();

        return (rand %99999).ToString().PadRight(5, '0');
    }

    private int GetRandomNumber()
    {
        using (var crypto = new RNGCryptoServiceProvider())
        {
            byte[] data = new byte[4];

            crypto.GetBytes(data);

            return Math.Abs(BitConverter.ToInt32(data, 0));
        }
    }
}

Solution


  • Use unsigned integers to avoid the Abs related issues



  • RightPad is incorrect, since shorter integers need leading zeros. Either use LeftPad or simply ToString("D5")



  • A bigger integer will reduce the bias.



Thus I'd rewrite the relevant code as:

byte[] buffer = new byte[sizeof(UInt64)];
cryptoRng.GetBytes(buffer);
var num = BitConverter.ToUInt64(buffer, 0);
var pin = num % 100000;
return pin.ToString("D5");

Code Snippets

byte[] buffer = new byte[sizeof(UInt64)];
cryptoRng.GetBytes(buffer);
var num = BitConverter.ToUInt64(buffer, 0);
var pin = num % 100000;
return pin.ToString("D5");

Context

StackExchange Code Review Q#125551, answer score: 6

Revisions (0)

No revisions yet.