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

Random Password Generator in C#

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

Problem

Security is a huge complicated subject. What I'm looking for here is three things:

  • The password generating algorithm isn't reversible, meaning that when someone looks at the source code of this application, that won't help them to break passwords made by it other than the fact that they can see the possibilities as strings there.



  • Could I somehow remove those possibilities as strings to make it more secure?



  • Am I overlooking any potential memory issues e.g. should I be zeroing/wiping memory in areas where the password is stored after?



enum Password_Options
{
    ALPHANUM,
    ALL
}
private string CreatePassword(int length, Password_Options options)
{
    const string valid = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890";
    const string valid_all = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890!@#$%^&*()_-=+{}:;\\<>?|,./`~[]'";
    if (options == Password_Options.ALPHANUM){
        StringBuilder res = new StringBuilder();
        byte[] random = new byte[1];
        RNGCryptoServiceProvider rProvider = new RNGCryptoServiceProvider();
        while (0 < length--){
            rProvider.GetBytes(random);
            res.Append(valid[random[0] % (valid.Length - 1)]);
        }
        return res.ToString();
    }
    else{
        StringBuilder res = new StringBuilder();
        byte[] random = new byte[1];
        RNGCryptoServiceProvider rProvider = new RNGCryptoServiceProvider();
        while (0 < length--)
        {
            rProvider.GetBytes(random);
            res.Append(valid_all[random[0] % (valid_all.Length - 1)]);
        }
        return res.ToString();
    }
}

private void button1_Click(object sender, EventArgs e)
{
    Password_Options po;
    if (radioButton1.Checked){
        po = Password_Options.ALL;
    }
    else{
        po = Password_Options.ALPHANUM;
    }
    textBox1.Text = CreatePassword((int)numericUpDown1.Value,po);
}

Solution

One way to avoid using constant strings is to use the char.IsLetterOrDigit method, just restrict the values mod 92 and add to 33 to get all printable characters to 125:

enum Password_Options
{
    ALPHANUM,
    ALL
}
RNGCryptoServiceProvider rProvider = new RNGCryptoServiceProvider();
private string CreatePassword(int length, Password_Options options)
{
    StringBuilder res = new StringBuilder();
    byte[] random = new byte[1];
    using (rProvider)
    {
        while (0 < length--)
        {
            char rndChar = '\0';
            do
            {
                rProvider.GetBytes(random);
                rndChar = (char)((random[0] % 92) + 33);
            } while (options == Password_Options.ALPHANUM && !char.IsLetterOrDigit(rndChar));
            res.Append(rndChar);
        }
    }
    return res.ToString();
}


According to MSDN you should always dispose of the rng provider after using it. One way is with a using block.

Code Snippets

enum Password_Options
{
    ALPHANUM,
    ALL
}
RNGCryptoServiceProvider rProvider = new RNGCryptoServiceProvider();
private string CreatePassword(int length, Password_Options options)
{
    StringBuilder res = new StringBuilder();
    byte[] random = new byte[1];
    using (rProvider)
    {
        while (0 < length--)
        {
            char rndChar = '\0';
            do
            {
                rProvider.GetBytes(random);
                rndChar = (char)((random[0] % 92) + 33);
            } while (options == Password_Options.ALPHANUM && !char.IsLetterOrDigit(rndChar));
            res.Append(rndChar);
        }
    }
    return res.ToString();
}

Context

StackExchange Code Review Q#157751, answer score: 5

Revisions (0)

No revisions yet.