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

PHP password encryption algorithm

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

Problem

I've written a password encryption algorithm in PHP, which (I think) is not very vulnerable to rainbowtable attacks. It's just that I don't have a lot of experience with encryptions, nor PHP. But from the knowledge I have I think this is a hashing algorithm which automatically adds salt, but I actually have no idea if this is true or not, so please tell me.

This is the code I use to create a password hash:

 $val) {
    $pass_chars[$key] = ord($val);
  }

  // Generate an array of random characters the size of $pass_chars
  for ($i = 0; $i  $val) {
    $pass_vals[$key] = $val + $pass_chars[$key + 1];
  }
  $pass_vals[$pass_len] = $pass_chars[$pass_len] + $pass_chars[0];

  // Create list of added $rand_vals values
  foreach ($rand_chars as $key => $val) {
    $rand_vals[$key] = $val + $rand_chars[$key + 1];
  }
  $rand_vals[$pass_len] = $rand_chars[$pass_len] + $rand_chars[0];

  // Add $rand_vals to $pass_chars
  foreach ($pass_chars as $key => $val) {
    $pass_chars[$key] += $rand_vals[$key];
  }

  // Add $pass_vals to $rand_chars
  foreach ($rand_chars as $key => $val) {
    $rand_chars[$key] += $pass_vals[$key];
  }

  // Create combined array
  $i = 1;
  foreach ($pass_chars as $key => $val) {
    $combined[$key * 2] = str_pad($pass_chars[$key], 3, "0", STR_PAD_LEFT);
    $combined[$i] = str_pad($rand_chars[$key], 3, "0", STR_PAD_LEFT);
    $i += 2;
  }

  // Print $combined as string
  echo implode($combined) . "\n";
?>


And this code to reverse it:

```
$val) {
$pass_chars_i[$key] = ord($val);
}

// Create list of added $pass_vals_i values
foreach ($pass_chars_i as $key => $val) {
$pass_vals_i[$key] = $val + $pass_chars_i[$key + 1];
}
$pass_vals_i[$pass_len_i] = $pass_chars_i[$pass_len_i] + $pass_chars_i[0];

// Remove extra '0's at the left in $hash_chars if there are any
foreach ($hash_chars as $key => $val) {
$hash_chars[$key] = ltrim($val, "0");
}

// Extract $rand_chars and $pass_chars from $hash_chars
$i = 1;

Solution

Security

I'm assuming this is for educational purposes. Otherwise, don't roll your own, use bcrypt.

From a quick look at it, it seems that the core of your algorithm basically boils down to this:

foreach character in password:
    character += random(33, 126);


There are more additions, but they do not seem to affect the basic principle.

This means that an attacker can gain at least some information about the password by looking at the encrypted password. An encrypted a will statistically have a smaller value than an encrypted z.

You can see this easily by adjusting the range from which random numbers are selected. Using for example mt_rand(33, 35);, encrypting az will result in this: 166253191254, 165253190253, 167254192254, etc. You can see that 166 is smaller than 191, 165 is smaller than 190, or 167 is smaller than 192.

Making the random range larger makes this problem less sever, but it still exists. z for example can reach values which other characters can never reach. So whenever you see 374, you know for sure that it's a z. When you see 167 you can be pretty sure that it's an a, and so on. This makes brute-force attempts significantly easier for an attacker.

Additionally, the size of the password is easily calculated given the encrypted password. This should not happen.

Also, note the documentation of mt_rand: Caution: This function does not generate cryptographically secure values, and should not be used for cryptographic purposes.

I haven't looked at the decryption code in-depth, so I'm not sure if there are more weaknesses, but there very well might be (I'm not yet 100% convinced that the original password is even needed for decryption).

Code

  • Your variable naming is very confusing. What's the difference between a val and a char? They seem to stand for the same thing? In that case, use the same name. If they stand for different concepts, use clearer names, or add comments.



  • Comments: Your comments basically describe what your code does. But I already know that from looking at the code. As this is relatively complicated code, I would actually like to have comments that describe why the code does what it does; what's the use of each of the for loops? What's the general idea of the algorithm? etc.



  • order of code blocks: your different foreach blocks seem to be randomly ordered. First, you work on the pass_chars, then on rand_chars, then on pass_chars/pass_vals again, then again on rand_chars/rand_vals. As these do not depend on each other yet, it would make more sense to first work on pass_, then on rand_.



  • With an input of pass, I get a warning: Notice: Undefined offset: 4 in /var/www/e.php on line 19

Code Snippets

foreach character in password:
    character += random(33, 126);

Context

StackExchange Code Review Q#101374, answer score: 3

Revisions (0)

No revisions yet.