Skip to main content
Code Review

Return to Answer

replaced http://crypto.stackexchange.com/ with https://crypto.stackexchange.com/
Source Link

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 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.

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.

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.

Source Link
yannis
  • 2.1k
  • 1
  • 19
  • 38

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.

lang-php

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