Is my code secure enough?

lostprophetpunk · August 7, 2008

I am making stuff for my site and I need to know if my login script is safe enough...

<?php

require_once('include.php');

$error = '';
$form = $_POST['submit'];
$username = mysql_real_escape_string( $_POST['username'] );
$password = mysql_real_escape_string( $_POST['password'] );

//$MD5password =  md5( $password );

if( isset($form) ) {
if( isset($username) && isset($password) && $username !== '' && $password !== '' ) {

$MD5password =  md5( $password );

$MD52password = md5( $MD5password );

$sql = mysql_query("SELECT * FROM `usersystem` WHERE username='$username' and
password='$MD52password';");

if( mysql_num_rows($sql) != 0 ) { //success

$_SESSION['logged-in'] = true;

header('Location: members.php');

exit;

} else { $error = "Incorrect login info"; }
} else { $error = 'All information is not filled out correctly';}
}

?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
"http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">

<head>

<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
<title>Login</title>

</head>

<body>

<form action="<?php $PHP_SELF; ?>" method="post" >
Username:
<input name="username" type="text" value="<?php echo "$username";?>" /><br /><br />
Password:
<input name="password" type="password" /><br />
<input name="submit" type="submit" value="Log In" />
</form>

<?php

echo "<br /><span style=\"color:blue\">$error</span>";

?>

</body>
</html>

If it is not safe, could you give me pointers on how to make if more safe?

darkfreaks · August 7, 2008

why do you md5 twice? thats a bit redundant

also you could use trim() and strip_tags to avoid XSS

corbin · August 8, 2008

As darkfreaks hinted, when ever you put out user input (such as the username in the script), you should sanitize it, but depending on the format of your usernames, it might not be necessary to filter the usernames.

darkfreaks, some people md5 twice as a method of hashing.

Think about if you're trying to brute force the hash of 'Corbin'.

d6deb917926e7370f0e64e0ef00d88d9

Now, to brute force, let's say we know the site has a minimum password length of 4, and a maximum password length of 16 (terrible policy, eh?). Let's also assume that users can enter passwords that are alphanumeric (a-zA-Z0-9).

Now, that's 62 possibilities per character, yes?

The amount of guesses to get the hash of Corbin would be immense. Now, imagine the number of guesses it would take to guess the hash of the hash.

With md5(Corbin), I'm trying to find a string which is equal to the hash of Corbin (except, I obviously wouldn't know it was Corbin for which I was looking). There are rainbow tables and other things that could easily find the original string based on md5(Corbin).

I'm going to abbreviate md5 to m because I'm that lazy. (Crazy!)

Anyway, if you have m(Corbin), you're trying to find Corbin. If you have m(m(Corbin)) you're trying to find m(Corbin), then you have to find Corbin.

Anyway, I'll let you think of the math part as I'm getting tired of typing, and I'm starting to ramble.

Edit:

Just realized it's not simply 62^len. It's:

62^len+62^(len-1)+62^(len-2) until len-n is 0.

Third_Degree · August 8, 2008

Some people don't stop at twice (me).

I frequently use encs like md5(sha1(md5(sha1())))

Third_Degree · August 8, 2008

also, you probably want to put an htmlentities() around the username for XSS prevention as darkfreaks said, but also like corbin said, it may not be nescessary.

waynewex · August 8, 2008

This would be better:

$MD5password =  md5("hjehduv4".$password."uhe!2hd89!" );

Always use a salt.

Dead6re · August 13, 2008

By passing a string through MD5 twice you increase the chances of collisions, it is better to use a salt that will prevent rainbow tables from being used.

scarhand · August 14, 2008

The amount of guesses to get the hash of Corbin would be immense. Now, imagine the number of guesses it would take to guess the hash of the hash.

Your logic makes absolutely no sense.

If someone has cracked the password it doesn't matter how many times you use the md5 function to protect it. Using one-way encryption one time is enough.

Third_Degree · August 14, 2008

Your logic makes absolutely no sense.

If someone has cracked the password it doesn't matter how many times you use the md5 function to protect it. Using one-way encryption one time is enough.

I don't think you know what you are talking about.

scarhand · August 14, 2008

Your logic makes absolutely no sense.

If someone has cracked the password it doesn't matter how many times you use the md5 function to protect it. Using one-way encryption one time is enough.

I don't think you know what you are talking about.

how do most crackers crack passwords? they use enormous text files containing words, and variations of words, in an attempt to match one up with a password. same goes for the md5...but figuring out exactly how to reverse the md5 one-way encryption can not be done. there is a reason why its called one-way encryption algorithm.

now if the cracker has access to your database, i'm afraid you have a lot bigger issues than only md5'ing a password once.

explain how i dont know what im talking about? you're the one who said you using functions such as "md5(sha1(md5(sha1())))".

Third_Degree · August 15, 2008

Your password is 'hello'. Someone has your hash. It is encrypted once. They perform a simple dictionary attack and your password is theirs in 10 seconds.

Your password is 'hello'. Someone has your hash. It is encrypted multiple times with multiple algorithms. They perform a dictionary attack and come up with nothing because hash databases don't carry the md5 of the sha1 of the md5 of the sha1 hash of 'hello'. They brute force. It takes them a day only to realize, wow, it's another hash that will take them another day to crack, and so on.

"now if the cracker has access to your database, i'm afraid you have a lot bigger issues than only md5'ing a password once."

This shows that you don't know what you are talking about. What is the point of encryption?

corbin · August 15, 2008

Because I'm too lazy to type out an argument:

I side with Third_Degree.

scarhand · August 18, 2008

Your password is 'hello'. Someone has your hash. It is encrypted once. They perform a simple dictionary attack and your password is theirs in 10 seconds.

Your password is 'hello'. Someone has your hash. It is encrypted multiple times with multiple algorithms. They perform a dictionary attack and come up with nothing because hash databases don't carry the md5 of the sha1 of the md5 of the sha1 hash of 'hello'. They brute force. It takes them a day only to realize, wow, it's another hash that will take them another day to crack, and so on.

"now if the cracker has access to your database, i'm afraid you have a lot bigger issues than only md5'ing a password once."

This shows that you don't know what you are talking about. What is the point of encryption?

You're missing the big picture. How did that person get the hash in the first place?

Like I said, if unauthorized people have access to your database and are harvesting account hashes, you have a lot bigger fish to fry. Anyone who wanted to screw you over and was in your database would probably not waste their time trying to crack hashes anyways.

dlate · August 18, 2008

The way i encrypt my password is by using 3 salts on an md5, a sha and another md5 password then substr it to 128 char hash and i store that in my database.

TheUkSniper · August 18, 2008

Like I said, if unauthorized people have access to your database and are harvesting account hashes, you have a lot bigger fish to fry. Anyone who wanted to screw you over and was in your database would probably not waste their time trying to crack hashes anyways.

Very true, but considering alot of people have a single password for all websites and programs they use, should'nt you as a code developer be protecting them further. It goes further than what a hacker is going to do with your site when he has email addresses and passwrods of potentially thousands of users.

dlate · August 18, 2008

Like I said, if unauthorized people have access to your database and are harvesting account hashes, you have a lot bigger fish to fry. Anyone who wanted to screw you over and was in your database would probably not waste their time trying to crack hashes anyways.

Very true, but considering alot of people have a single password for all websites and programs they use, should'nt you as a code developer be protecting them further. It goes further than what a hacker is going to do with your site when he has email addresses and passwrods of potentially thousands of users.

Exactly, this is what happened with stage6 they didnt hash there passwords properly and alot of people got there password exposed.

Sign In

Is my code secure enough?

Recommended Posts

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Link to comment

Share on other sites

Important Information