1

Is my PHP authentication script secure? I noticed that the hashes start with the first two letters of the username. Could there be a security flaw using crypt() in such a way?

<?php

// Credentials :
// admin / P4ssW0rd
// j.doe / r0x0r

$cred = array(
    'admin' => 'adkFV/7Pa.Em.',
    'j.doe' => 'j.4AzOhv10e1M'
);

$salt = 'abcdefg';
$user = $_POST['login'];
$pass = $_POST['pass'];

if (isset($cred[$user]) && crypt($salt . $pass, $user) == $cred[$user]) {
    echo 'Access granted';
} else {
    echo 'Access denied';
}
user46381
  • 11
  • 1
  • 2

2 Answers2

4

Your script is atrocious in several ways:

  • As was pointed out by @Corneliux, PHP's crypt() function uses the old DES-based crypt() function from the Unix world (precisely, what was used in Unix 20 years ago). It will use only the first 8 letters of the provided password, ignoring the rest. In your case, the first 7 letters are the contents of your $salt variable, not of the password. This means, in practice, that a user will be granted admin access as long as what he enters as password begins with a 'P'.

  • Even if crypt() used a decent password hashing function, you would still use it wrong. In PHP's crypt() function, you are supposed to provide the salt as second parameter; you don't glue it with the password; right now, you use the user name as salt.

  • Your $salt variable is not a salt anyway. The point of a salt is to change; ideally, each password hashing instance should have its own salt value (that is, one new salt for every user, and a new salt each time a user changes his password as well). An hardcoded salt value, always the same for everybody, is the exact opposite of what a salt is supposed to be.

  • Writing the plain passwords of users in the comments is, let's say, unwise.

Do yourself a favour: use a recent enough PHP (5.5.0 at least) and call password_hash(). That call will use bcrypt, which is as fine as you can realistically get. Don't fiddle with salts: just let the function generate a random salt internally; it will also store the salt encoded in the resulting hash value, which is fine.

Community
  • 1
Thomas Pornin
  • 322,884
  • 58
  • 787
  • 955
  • There's also a [compatibility library](https://github.com/ircmaxell/password_compat) from the same author to make the new password hashing API available to older PHP versions. This only requires PHP 5.3.7 or later. – Fleche Jun 11 '14 at 13:01
0

As stated by the PHP documentation, crypt() only takes the first 8 characters of the string:

The standard DES-based crypt() returns the salt as the first two characters of the output. It also only uses the first eight characters of str, so longer strings that start with the same eight characters will generate the same result (when the same salt is used).

So your salt weakens the whole security here. If it's less than 8 characters, then only the first (8-len(salt)) of the user's password will be needed to authenticate. If it's 8 or more characters, it's open bar.

Corneliux
  • 207
  • 1
  • 4