What are the weaknesses of this user authentication method?

Question

I'm developing my own PHP framework. It seems all the security articles I have read use vastly different methods for user authentication than I do so I could use some help in finding security holes.

Some information that might be useful before I start. I use mod_rewrite for my MVC url's. Passwords are encrypted with 24 character salt unique to each user. mysql_real_escape_string and/or variable typecasting on everything going in, and htmlspecialchars on everything coming out.

Step-by step process:

Top of every page:

session_start();
session_regenerate_id();

If user logs in via login form, generate new random token to put in user's MySQL row. Hash is generated based on user's salt (from when they first registered) and the new token. Store the hash and plaintext username in session variables, and duplicate in cookies if 'Remember me' is checked.

On every page, check for cookies. If cookies set, copy their values into session variables. Then compare $_SESSION['name'] and $_SESSION['hash'] against MySQL database. Destroy all cookies and session variables if they don't match so they have to log in again.

If login is valid, some of the user's information from the MySQL database is stored in an array for easy access. So far, I've assumed that this array is clean so when limiting user access I refer to user.rank and deny access if it's below what's required for that page.

I've tried to test all the common attacks like XSS and CSRF, but maybe I'm just not good enough at hacking my own site! My system seems way too simple for it to actually be secure (the security code is only 100 lines long). What am I missing?

Edit: For calling functions from the controller, anything that uses anything other than SELECT queries will require $_POST data to confirm a delete, for example, in addition to the user rank requirements.

I've also spent alot of time searching for the vulnerabilities with mysql_real_escape string but I haven't found any information that is up-to-date (everything is from several years ago at least and has apparently been fixed). All I know is that the problem was something to do with encoding. If that problem still exists today, how can I avoid it?

The encrypt function I borrowed from somewhere and modified:

public function encrypt($str, $salt = NULL) {
    if ($salt == NULL) $salt = substr(md5(uniqid(rand(), true)), 0, 24);
    else $salt = substr($salt, 0, 24);
    return $salt.sha1($salt.$str);
}

Answer 1

Do yourself a favour and use a standard library for hashing your passwords.

Because security tends to be a lot more complicated and with more invisible screw up possibilities than most programmers could tackle alone, using a standard library is almost always easiest and most secure (if not the only) available option.

The standard library:
Take a look at: Portable PHP password hashing framework: phpass and make sure you use the CRYPT_BLOWFISH algorithm if at all possible.

example of code using phpass (v0.2):

require('PasswordHash.php');

$pwdHasher = new PasswordHash(8, FALSE);

// $hash is what you would store in your database
$hash = $pwdHasher->HashPassword( $password );

// $hash would be the $hashed stored in your database for this user
$checked = $pwdHasher->CheckPassword($password, $hash);
if ($checked) {
    echo 'password correct';
} else {
    echo 'wrong credentials';
}

PHPass has been implemented in some quite well known projects:

• phpBB3
• WordPress 2.5+ as well as bbPress
• the Drupal 7 release, (module available for Drupal 5 & 6)
• others

The good thing is that you do not need to worry about the details, those details have been programmed by people with experience and reviewed by many folks on the internet.

Whatever you do if you go for the 'I'll do it myself, thank you' approach, do not use MD5 anymore. It is a nice hashing algorithm, but utterly broken for security purposes.

Currently, using crypt, with CRYPT_BLOWFISH is the best practice.
CRYPT_BLOWFISH in PHP is an implementation of the Bcrypt hash. Bcrypt is based on the Blowfish block cipher, making use of it's expensive key setup to slow the algorithm down.

For more information on password storage schemes, you could also read Jeff`s blog post about it: You're Probably Storing Passwords Incorrectly

Answer 2

I don't see any real prevention for CSRF attacks in there. CSRF is (from my understanding) the most commonly overlooked/screwed up vulnerability in websites. Many of the big guys have been hacked via CSRF vulnerabilities.

The current best solution to this problem is using synchronizer tokens.

You had also better be very religious about escaping/encoding user input properly. Things that are inserted into the database must be escaped, and things output back to the user have to be encoded based on their context: CSS, HTML, JavaScript, etc all have to be encoded differently.

Furthermore, you shouldn't combine SHA-1 with MD5. While SHA-1 is stronger than MD5, you are essentially weakening it by combining it with MD5. Additionally, neither of these algorithms are recommended for use in new applications. For the time being you should be using SHA-256 (it is available out-of-the-box in PHP).