3

Recently we outsourced some work for a website. While reviewing this code I came across the password hash function.

I am in no way a security expert besides some basic knowledge (hence outsourcing this), Nontheless it raised some red flags in my head and wanted to get this confirmed/debunked.

the complete function (in C#)

protected static string CreatePasswordHash(string pwd)
{
    return ByteArrayToString(new SHA1CryptoServiceProvider().ComputeHash(Encoding.ASCII.GetBytes(ByteArrayToString(new MD5CryptoServiceProvider().ComputeHash(Encoding.ASCII.GetBytes("GHYETAZ86E9O"))) + pwd)));
}

public virtual string GetPasswordhash(string password)
{
    return CreatePasswordHash(password);
}

I feel the like this has the following issues:

  • It uses Md5 (to hash the salt. Which I don't really see the use of, This seems to imply it isn't any more secure either)
  • It uses a static hardcoded salt that is the same for everyone (which will generate the same hashes for users with the same password)
  • No double salt (ties in with the above)
  • It uses SHA1 instead of SHA2 (We don't need to stick to sha1 for any compatibility reasons or anything)

Now as far as i'm aware SHA1 isn't truly unsecure (yet) but has already had collisions happening, and isn't completely secure anymore.

Are these security concerns legit? Or is this a proper hashing implementation.

The ability to create an account is public and can be done by anyone who visits the website.

I hope this is the correct SE to ask this, but was doubting between here and code review. This seemed the better option. Let me know if I was wrong

Remy
  • 131
  • 3
  • 1
    There's a related question here that might help: https://security.stackexchange.com/questions/11205/what-is-the-problem-with-chain-hashing – Philip Rowlands Apr 19 '19 at 12:29
  • 3
    tl;dr you're correct on all of your concerns, but additionally a fast cryptographic hash (even unbroken like SHA256) is not sufficient to hash passwords. – AndrolGenhald Apr 19 '19 at 13:43
  • 3
    Outsource it again starting from scratch. Blacklist the first company from future contracts. That's such a basic security blunder there is no chance in hell they managed to do the rest of the site securely. – Ben Apr 19 '19 at 13:58
  • 5
    Related regarding outsourcing such stuff: *"If you want, I can store the encrypted password." A Password-Storage Field Study with Freelance Developers* - https://net.cs.uni-bonn.de/fileadmin/user_upload/naiakshi/Naiakshina_Password_Study.pdf – Steffen Ullrich Apr 19 '19 at 16:28

0 Answers0