0

I need to create a random Password but that it complies with some specific parameters:

Must have a mayus

Must have numbers

It must have special characters.

It can not contain the following strings "123", "12345", "56789", "123456789", "321", "54321", "987654321", "qwerty", "asdf", "zxcv", "poiuy", "lkjhg", "mnbv"

Among other.

I already did it with the following code, but it throws me an error of StackOberflowException, of what another way I can achieve it or what would be the solution to this error?

public static string CrearPassword(int longitud,string usuario)
    {
        string caracteres = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNÑOPQRSTUVWXYZ1234567890ñÑ-_¿.#¡";
        StringBuilder res = new StringBuilder();
        Random rnd = new Random();
        while (0 < longitud--)
        {
            res.Append(caracteres[rnd.Next(caracteres.Length)]);
        }

        while (ValidPassword(res.ToString(), usuario)== false)
        { 
            return CrearPassword(13,usuario);
        }
        return res.ToString();
    }

    public static bool ValidPassword(string pass, string usuario)
    {
        try
        {
            Match matchLongitud = Regex.Match(pass, @"^\w{8,15}\b");
            Match matchNumeros = Regex.Match(pass, @"\d");
            Match matchEspeciales = Regex.Match(pass, @"[ñÑ\-_¿.#¡]");
            Match matchMayusculas = Regex.Match(pass, @"[A-Z]");
            Match matchAdmin = Regex.Match(pass, @"admin");
            Match matchContraseña = Regex.Match(pass, @"contraseña");
            Match matchNombreUsuario = Regex.Match(pass, usuario);
            var valoresProhibidos = new List<string>() { "123", "12345", "56789", "123456789", "321", "54321", "987654321", "qwerty", "asdf", "zxcv", "poiuy", "lkjhg", "mnbv" };

            if (!matchNumeros.Success)
                return false;
            else if (!matchLongitud.Success)
                return false;
            else if (!matchEspeciales.Success)
                return false;
            else if (!matchMayusculas.Success)
                return false;
            else if (matchAdmin.Success)
                return false;
            else if (matchContraseña.Success)
                return false;
            else if (matchNombreUsuario.Success)
                return false;
            else
            {
                foreach (string valor in valoresProhibidos)
                {
                    if (pass.Contains(valor))
                    {
                        return false;
                    }
                }
            }

            return true;

should validate and return the password, but pulls error from SystemStackOverflowException enter image description here

R.J. Dunnill
  • 2,049
  • 3
  • 10
  • 21
  • When you debugged it, what did your stack trace look like? See also https://stackoverflow.com/questions/52107850/prevent-system-stackoverflowexception-when-recursively-calling-a-method . – mjwills Jun 13 '19 at 21:50
  • 1
    You shouldn **not** use `Random` for password generation. Use [RNGCryptoServiceProvider](https://learn.microsoft.com/en-us/dotnet/api/system.security.cryptography.rngcryptoserviceprovider?view=netframework-4.8) instead. – itsme86 Jun 13 '19 at 21:54
  • I think a good random password would come from `new string(caracteres.OrderBy(c => Guid.NewGuid()).ToArray()).Substring(longitud)` – dcg Jun 13 '19 at 21:55
  • 2
    @dcg No. GUIDs are good at being unique, but not necessarily random. – itsme86 Jun 13 '19 at 21:55
  • @itsme86 Then changing it by `random.NextDouble()` would do the trick? – dcg Jun 13 '19 at 22:00
  • Seems to be stack overflow due to recursion? I'd pull the validation/retry out of the creation method into a separate loop. – Ben Jun 13 '19 at 22:08
  • @dcg I can't do a better job describing this than Eric Lippert, so I'll just link to a blog post he wrote about this: https://blogs.msdn.microsoft.com/ericlippert/2012/05/07/guid-guide-part-three/ – itsme86 Jun 13 '19 at 23:24
  • If you insist on using off-label tools to generate passwords, `Path.GetRandomFileName` is less awful than using `Random` or `Guid` (because it's documented as being cryptographically strong). Mind you, Microsoft has plenty of password generation functions inside their various authentication frameworks, if you don't mind taking a dependency. – Brian Jun 14 '19 at 12:51

2 Answers2

4

Your stack overflow is because you have an unbounded recursion. Remember, a recursive program ABSOLUTELY MUST have the property that the recursive step is a smaller problem. You have given your recursive step a problem of the same size, and it might never stop.

The correct way to write your program is to write two methods:

public static string CreateValidPassword(int longitud, string usuario)
{
  while(true)
  {
    var password = CreateRandomPassword(longitud, usuario);
    if (ValidPassword(password)) 
      return password;
  }
}

public static string CreateRandomPassword(int longitud, string usuario)
{ 
  // Create a random password **CORRECTLY THIS TIME**
}

Your code to create the random password is bad in many ways, but your question was about solving the stack overflow. This will solve the stack overflow. Work on improving your random password generator in its own method.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
0

The problem you are having is related to two things:

First of all, you are using recursion to generate your password, and that I can tell for experience it can lead to a nasty stack overflow, but the reason you are getting this stack overflow is because you are never generating a valid password probably thus the function never ending, and the reason is, every time you call your function CrearPassword you are generating a new Random, and it probably takes the same time, since the random function uses the time as seed you are probably getting the same seed and always getting an invalid password.

What can you do then?

Well for starters if you insist into using the random, take it out of the function, make it a global variable, and just call the rnd.Next from your function. Secondly, you should make another function that deals with the Creation and rename that CreatePassword as a SuggestPassword. Just to put it in code this is my suggestion.

public void CreateRandomPassword()
{
    while(true)
    {
        string pass = CrearPassword(8, "user");
        if(ValidPassword(pass, "user"))
        {
            break;
        }
    }
}
nalnpir
  • 1,167
  • 6
  • 14