How Secure is my PHP login?

[VengefulWrath]

Hi all!

I'm wondering if you could look at my login code and tell me how secure it is. I know it seems to work pretty well, I'm just not very good at picking out security hazards.

Main login.php page:
A simple form which submits to itself to check for both fields filled in and checks to see if both the username and password correspond to a user in the database. Both values are md5 encrypted.

If the credentials are correct, the user is redirected to set.php which sets the session and cookie.

set.php:

if(isset($_GET['usr'])){
  $username = $_GET['usr'];
  $password = $_GET['pss'];

  $hour = time() + (3600*24*30); 
  
  setcookie('uid', md5($username), $hour);
  setcookie('pss', $password, $hour);
  
  $_SESSION['ses_user'] = md5($username);
  $_SESSION['ses_pass'] = $password;
}
 // Redirects them to index page after setting session and cookie
echo "<script>location.href='../manage/'</script>";

Every page requiring username and pass:
I then have every page which requires a username include the following code at the top of the file:

session_start(); 

// A function to check whether a value is md5  (kinda)
function CheckMd5($check){
	$valid = preg_match('/^[a-z0-9]{32}$/',$check);
	if($valid != 1){
		setcookie("uid", "", time()-3600);
		setcookie("pss", "", time()-3600);
		session_start();
		session_destroy();

		echo("<script>location.href='login.php?ref=edit'</script>");
	}
}
// connect.php connects to MySQL database
require("connect.php");

$access = 0;
if(isset($_COOKIE['uid'])){
	$username = $_COOKIE['uid'];
	CheckMd5($username);
	$pass = 	$_COOKIE['pss'];
	CheckMd5($pass);
	$check = 	mysql_query("SELECT * FROM users WHERE username_enc = '$username'") or die (mysql_error());
	while($info = mysql_fetch_array($check)){
		if($pass == $info['password']){
			$access = 1;
		}
		
		$username = $info['username'];
	}
}
if(isset($_SESSION['ses_user'])){
	$username = $_SESSION['ses_user'];
	CheckMd5($username);
	$pass = 	$_SESSION['ses_pass'];
	CheckMd5($pass);
	$check = 	mysql_query("SELECT * FROM users WHERE username_enc = '$username'") or die (mysql_error());
	while($info = mysql_fetch_array($check)){
		if($pass == $info['password']){
			$access = 1;
		}
		
		$username = $info['username'];
	}
}

if($access != 1){
	echo "<script>location.href='login.php?ref=edit&id=".$_GET['id']."'</script>";
}else{

// Page Content Goes Here

}

Any tips or reassurance would be appreciated. :icon_eek:

[FreddieBambino]

Why the cookie thing?

Why not just
Form that sends post username and password.
Then make variable of username and password with trimming and stripping tags for exampe:
$username = strip_tags(trim($_POST));
Then check if there are dangerous characters, for example make sure that only alphanumeric letters:
if(!ctype_alnum($username)){
$deny_access = TRUE;
$output = "Those characters are not allowed";
}

if(!deny_access)){
//for second layer of security escape the strings
$username = $db_connection -> real_escape_string($username);
//check database for username and password;
//if success, make session, else redirect to login with output message. If the login form and the php processing is on the same page you just echo out $output here, if it is on another page, then instead of putting error message into variable you make a session with the eroror message to be able to carry it to the next page, and then destroy the session.
}

[VengefulWrath]

Well, I have the cookies set so that they don't have to log in every time they come to the page.

Do you know if the way I have it is secure enough? I'm not really looking to rework the whole process unless it is grossly insecure. :/

Are you suggesting that I have stronger check for malicious characters or code through the form?

[makman99]

Yes, you definitely need checks for bad characters. I see none in the code posted.
You should have them in both the $_GET area and the $_COOKIE area. Since both can be modified, you need to clear bad characters. As it is now, it's easily subject to SQL injection...

[VengefulWrath]

alright, I'll add some checks in there. I was just assuming that since my gets looked threw errors if they received anything other than md5 hashes, that my login was secure. but I guess that isn't protecting the main login page or the page which sets the cookie initially.

I'm pretty new to this PHP thing, are there any checks that are normally used and thought as standard?

[diafol]

sanitize ALL input data.

[Stefano Mtangoo]

sanitize ALL input data.

It is huge process actually. I plan to do little series on security by the end of this month, God willing (Not expert at all but learning) But here is my suggestion on angles to check:
1. As ardav pointed out, always suspect user - Sanitize and validate ALL external data
2. Secure your session and site against CSRF, XSS and other common attacks
3. Escape all outputs
4. Guard your Database against SQL injection
5. Add DoD, that is in case one thing is compromised (like session variable) then intruder can still be delayed to get in
6. Log all login attempts and if username applies lock after several attempts and alert user with unlock link. If user does not exist, send email to admin.

Actually there are more to that but those are the one I can think for a moment

[TechySafi]

You use only md5() ? Though me and my friends aren't pro, we can crack 90%~98% of passwords hashed with only md5() !

I'm not talking about your script, I'm talking about your password safety.

I use something like
$one=24TngAO5%#@;
$two=.,8&254GDkhkk;
$salt1=md5($one);
$salt2=md5($two);
$final=md5($salt1.$pass.$salt2);

Now no fuckin rnbw table can crack it! Even with 40 billion hash, you won't get a single password because salts are also hashed.
For more secure clients I use even stronger algorithm, I use sha512 instead of md5 in $final variable.

[TechySafi]

But clever crackers can figure out which part is actually password and which part is salted....soo we should use random thing and sha!

[VengefulWrath]

That salt thing makes a lot of sense, especially since I can't control what passwords users create. If their password is like pass123, that can be cracked pretty quickly by searching an md5 database.

I'm also going to deploy as many of the items on evsteve's checklist as possible.

I've done some searching on Google for PHP security resources, but most of the stuff I've gathered from a hodgepodge of sources. Do any of you have a book or site that you could recommend for me to learn more about PHP security in particular?

[TechySafi]

Nah can't remember any but go search on library.nu. You will get a lot of books on php security and stuff

[diafol]

I bought a book a few years back:

"Essential PHP Security" Chris Shiflett (O'Reilly) isbn0-596-00656-x

Good

[FreddieBambino]

I use sessions instead of cookies(all though strictly speaken it is the same thing.).

To use a session you do this:

On top of all pages you put:
session_start();

then to make a session you do for example:
$_SESSION = 8;

then if you want to print it on screen you do:
echo $_SESSION;

And yes you should have stronger filtering against input. Both you should make sure it is not a xss attack(for example javascript session script) you do this by $variable = strip_tags($variable);
And then you need to escape variables to prevent mysql injection.

[Stefano Mtangoo]

I bought a book a few years back:

"Essential PHP Security" Chris Shiflett (O'Reilly) isbn0-596-00656-x

Good

Yeah, Shiflett is one of the experts on the field. I have learned a lot on his web. Here it is with some helpful security websites:
http://shiflett.org/
http://phpsec.org/projects/guide/

[VengefulWrath]

Thanks for all your help guys, I just purchased "Essential PHP Security" which should help with a lot of this.

[minitauros]

Sha1 is safer than md5, and it's also built in in PHP, so why not use sha1? :) Where md5 returns a 32 character long encrypted hash, sha1 returns a 40 character long encrypted hash. It may not seem like that's a lot of difference, but it is!

[Stefano Mtangoo]

Sha1 is safer than md5, and it's also built in in PHP, so why not use sha1? :) Where md5 returns a 32 character long encrypted hash, sha1 returns a 40 character long encrypted hash. It may not seem like that's a lot of difference, but it is!

I use Sha512 and 128 character length is not a problem to me :)

[diafol]

You can compare output of each here: http://diafol.org/md5.php