How Secure is this?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

How Secure is this?

Post by blacksnday »

Working on a user Auth/Login system and been thinking along these lines:
YES YOU WOULD WANT TO PROTECT GET AND POST DATA.

Lets start with making a default session function
that will always be the values for session data unless
the user correctly logs in:

Code: Select all

function session_defaults() { 
$_SESSION['logged'] = 'false'; 
$_SESSION['userid'] = 'false'; 
$_SESSION['password'] = 'false';
$_SESSION['username'] = 'Guest'; 
}
Now lets check if user is Logged In:

Code: Select all

if($_SESSION['loginname']=='Guest' || $_SESSION['logged']=='false' || $_SESSION['userid']=='false' || $_SESSION['password']=='false') 
	{ 
 		echo "
 		<div align='center'>
 		<h1>Login</h1> 
 		<p><form method='post' action=''> 
   		Username: <input type='text' name='username' /><br /> 
   		Password: <input type='password' name='password' /><br /> 
   		<input type='submit' name='submit' value='Log in' /> 
 		</form></p> 
		 </div>
		";
	 	exit; 
	}
Now lets check the form input values to see if it is a valid user

Code: Select all

if ($_POST['submit'])&& $_POST['username'] && $_POST['password'])
{
  $username = $_POST['username'];
  $password = sha1($_POST['password']);
	
	$sql = mysql_query("SELECT * FROM table WHERE name='$username' AND pass='$password'"); 
	$login_check = mysql_num_rows($sql);

	if($login_check > 0)
	{
		while($row = mysql_fetch_array($sql))
		{
		$_SESSION['displayname']= $username;
		$_SESSION['loginname'] 	= sha1($username); 
		$_SESSION['password'] 	= sha1($password);
		$_SESSION['logged']   	= sha1('yes');
		$_SESSION['userid'] 	= sha1($row['userid']);
		}
	}
	else if ($login_check == 0) 
	{ 
 		session_defaults();

		echo "<p align='center'>Please Try Again!</p>";
	}
}
Notice how when the user correctly logs in, all the SESSION data is turned
into SHA1? Except for the 'displayname' which would be used to
say hi to the user or something......

whatcha think?
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

someone could, theoretically, choose a password of "false." I'd suggest using the actual false and true boolean values. Make sure to use === then ;)

edit: why are you sha'ing all 4 session values? What's the point; why keep their password in the session? Why keep their login name in the session? You should only need their user id.

Btw, you've double sha'd their password when you stick it into the session. ;)
Last edited by feyd on Tue Feb 14, 2006 3:22 pm, edited 1 time in total.
User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

Post by blacksnday »

feyd wrote:someone could, theoretically, choose a password of "false." I'd suggest using the actual false and true boolean values. Make sure to use === then ;)
good thing i posted here.
I never thought of that :D
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

I edited my post, fyi.
Ree
Forum Regular
Posts: 592
Joined: Fri Jun 10, 2005 1:43 am
Location: LT

Post by Ree »

Why SHA1 the session vars? Only to be sure no kid can read them on a shared host?
User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

Post by blacksnday »

Ree wrote:Why SHA1 the session vars? Only to be sure no kid can read them on a shared host?
exactly.
Im writing this for a script i plan to release to the public
and would like to avoid possible shared host problems.

I also protect the logged user by using the
Challenge/Response in way of Generating/Comparing random
Session Value on Each page load.
But that code is for a different post if I ever need to :P

Storing all the above values into sessions isn't really needed.
I do agree.
Even if I stored just the UserId into a session... I wouldn't
rest easy unless that was SHA1 as well........
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Unfortunately, doing an sha1 of a User ID is most often pointless. User ID's are, generally by nature, sequencial and predictable. Now, if your User ID's were generated from near true random data, you'd at least have something to stand on, but an sha of that may reduce entropy making it less secure.

If you want more secure hashing, use my SHA256 class, or wait for me to release the SHA512 or SHA1024 classes.

Something else to consider: you may want to create your own sessions as the user may have sessions already set, which you may overwrite or wipe out with yours. Granted, you could change the session name (this should be a configuration setting really)
User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

Post by blacksnday »

feyd wrote:Unfortunately, doing an sha1 of a User ID is most often pointless. User ID's are, generally by nature, sequencial and predictable. Now, if your User ID's were generated from near true random data, you'd at least have something to stand on, but an sha of that may reduce entropy making it less secure.

If you want more secure hashing, use my SHA256 class, or wait for me to release the SHA512 or SHA1024 classes.
what was I thinking when i said sha1 of userid's?
*i will slap myself*
feyd wrote: Something else to consider: you may want to create your own sessions as the user may have sessions already set, which you may overwrite or wipe out with yours. Granted, you could change the session name (this should be a configuration setting really)
I remember a few months ago how I found that out!
When I gave my shoutbox code to a friend.... and it kept
logging them out of their website :P haha..
took me 4ever to figure out what the cause was.
But a lesson well learned !
User avatar
shiflett
Forum Contributor
Posts: 124
Joined: Sun Feb 06, 2005 11:22 am

Re: How Secure is this?

Post by shiflett »

blacksnday wrote:

Code: Select all

$username = $_POST['username'];
$password = sha1($_POST['password']);

$sql = mysql_query("SELECT * FROM table WHERE name='$username' AND pass='$password'");
$login_check = mysql_num_rows($sql);
I always tell people that my favorite username is:

chris' --

Your query illustrates why. I can log in as chris without knowing the password. (There are other attacks that would let me log in without even knowing a valid username.)

(By the way, you might not want to name your MySQL result set $sql. That's just confusing.)
User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

Re: How Secure is this?

Post by blacksnday »

shiflett wrote:
blacksnday wrote:

Code: Select all

$username = $_POST['username'];
$password = sha1($_POST['password']);

$sql = mysql_query("SELECT * FROM table WHERE name='$username' AND pass='$password'");
$login_check = mysql_num_rows($sql);
I always tell people that my favorite username is:

chris' --

Your query illustrates why. I can log in as chris without knowing the password. (There are other attacks that would let me log in without even knowing a valid username.)

(By the way, you might not want to name your MySQL result set $sql. That's just confusing.)
As I said in my first post:
blacksnday wrote: YES YOU WOULD WANT TO PROTECT GET AND POST DATA.
Which I do of course, however for this post I didnt feel the need to share
what and how I protect GET and POST
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Re: How Secure is this?

Post by Roja »

blacksnday wrote: As I said in my first post:
blacksnday wrote: YES YOU WOULD WANT TO PROTECT GET AND POST DATA.
Which I do of course, however for this post I didnt feel the need to share
what and how I protect GET and POST
You cannot reasonably expect a complete answer to the question of "How secure is this" while excluding the entire scope of filtering input.
User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

Re: How Secure is this?

Post by blacksnday »

Roja wrote:
blacksnday wrote: As I said in my first post:
blacksnday wrote: YES YOU WOULD WANT TO PROTECT GET AND POST DATA.
Which I do of course, however for this post I didnt feel the need to share
what and how I protect GET and POST
You cannot reasonably expect a complete answer to the question of "How secure is this" while excluding the entire scope of filtering input.
Considering I let it known right away that Form Input should be protected
I have no clue why that would cause an exclusion to a proper answer
about the other aspects of coding.

Just reminds me why I rarely post here anymore.
Alot of people are so anal and quick to nit-pick over subjects even when
the original person attempted to exclude an area that would cause such an event to happen.
User avatar
shiflett
Forum Contributor
Posts: 124
Joined: Sun Feb 06, 2005 11:22 am

Post by shiflett »

From what are you protecting GET and POST data? :-)

Other problems with the code you've posted:

1. Your conditional statement fails if the session data is not initialized. You have a function for this, but you've not illustrated how this is guaranteed to happen before your conditional statement. I assume a GET request sent to the action of your form will subvert this.

2. Hashing session variables has no value. Unset them if you don't need them.

Instead of asking for general comments (whatcha think?) and acting as if you're already aware of vulnerabilities, perhaps you should explain what you're trying to do.
User avatar
blacksnday
Forum Contributor
Posts: 252
Joined: Sat Jul 30, 2005 6:11 am
Location: bfe Ohio :(

Post by blacksnday »

shiflett wrote:From what are you protecting GET and POST data? :-)
From another function I have not posted
shiflett wrote: 1. Your conditional statement fails if the session data is not initialized. You have a function for this, but you've not illustrated how this is guaranteed to happen before your conditional statement. I assume a GET request sent to the action of your form will subvert this.
Correct. I just assumed (which is my fault) that other people would realize
I have initiated sessions already
shiflett wrote: 2. Hashing session variables has no value. Unset them if you don't need them.
Feyd suggested same in an earlier post.
I agree!
shiflett wrote: Instead of asking for general comments (whatcha think?) and acting as if you're already aware of vulnerabilities, perhaps you should explain what you're trying to do.
Correct and is my own fault again.
I actually have several other functions used to
Validate, Challenge-Response, Sanitize, etc... that I
chose not to fully list here as I was only concerned about the way to handle a user
before and after login.
YES I SHOULDVE CHOOSE A DIFFERENT POST TITLE TO CLARIFY!
I agree ;x

If I make another post in the future that concerns questions of some code
I have written, I will be be a bit more detailed in the reponse I am hoping to recieve.

Thanks for the advice from everyone who gave it :)
User avatar
shiflett
Forum Contributor
Posts: 124
Joined: Sun Feb 06, 2005 11:22 am

Post by shiflett »

blacksnday wrote:
shiflett wrote:From what are you protecting GET and POST data? :-)
From another function I have not posted.
Must be a dangerous function. :-)
blacksnday wrote:I just assumed (which is my fault) that other people would realize I have initiated sessions already.
I know what you're saying in theory, but I want to stress that such assumptions can easily be problematic. How certain are you that there's no way to avoid having these session variables initialized? I'm not suggesting that there's a way, but it's worth at least trying the attack I suggested (sending a GET request to the URL identified as the action of the form).

To give an analogy, golfers often assume they can make a shot when the ball sits very near the hole. Casual golfers will pick up the ball, count it as a stroke, and move to the next hole. There's no need to waste time taking the shot - they just assume they'll make it.

This works fine in golf, but it doesn't apply very well to security.
blacksnday wrote:If I make another post in the future that concerns questions of some code I have written, I will be be a bit more detailed in the reponse I am hoping to recieve.
Sorry if you didn't get the answers you sought, but hopefully you found some value in the responses here.
Post Reply