Skip to main content
Code Review

Return to Revisions

2 of 2
replaced http://crypto.stackexchange.com/ with https://crypto.stackexchange.com/

This part doesn't make sense to me:

private function crypt_pass($pass)
{
 if (CRYPT_BLOWFISH == 1)
 {
 $salt = SHA1('acssalty457');
 $blowfish_salt = "\2ドルa\$07\$".substr($salt, 0, CRYPT_SALT_LENGTH);
 return crypt($pass, $blowfish_salt);
 }
}

Functions that only return on condition are a bad idea. You should return something anyway, even if it's false. Now for the specific problem, what if CRYPT_BLOWFISH != 1? It's ok to want to use blowfist, but why cripple the library for everyone else? You could rewrite as:

private function crypt_pass($pass) {
 $salt = "acssalty457";
 if (CRYPT_BLOWFISH == 1) {
 $blowfish_salt = "\2ドルa\$07\$" . substr($salt, 0, CRYPT_SALT_LENGTH);
 return crypt($pass, $blowfish_salt);
 }
 return sha1($pass . $salt); 
}

Notice that I don't hash the salt? That's because security wise that's useless. You can ask around at Cryptography Stack Exchange for more. If you want to make the sha1 crypted pass a little bit harder you could do something weird like:

return sha1(strrev($salt) . $pass . $salt);

I'm using sha1 as an example of a natively included function, you can use any hash algorithm you like.

yannis
  • 2.1k
  • 1
  • 19
  • 38
default

AltStyle によって変換されたページ (->オリジナル) /