Page 1 of 1

basic security

Posted: Mon Jun 11, 2007 5:01 am
by big_boss
feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


Is the following function enough for secure login:

Code: Select all

function checkUser(){
	if ((!isset($_SESSION['validUser'])) || ($_SESSION['validUser'] != true)){
		header('Location: login.php');// if user not logged they are redirected to the login page
	}
}
every file with protected data starts with:

Code: Select all

<?php
	checkUser(); //checking if user logged in
?>
The login function:

Code: Select all

session_start();

function loginUser($user,$pass){
	$errorText = '';
	$validUser = false;

	require 'details.php';//details for database 
  
$con = @mysql_connect($dhost, $duser, $dpass);
if (!$con)
  {
  die('Could not connect: ' . mysql_error());
  }

@mysql_select_db($databasename, $con);


$result = @mysql_query("SELECT username, password from users");
$row = @mysql_fetch_array($result);
$usern = $row['username'];
$passw = $row['password'];
	 // check password
            if ($usern == $user && $passw == $pass)
			{
            	$validUser= true;
            	$_SESSION['userName'] = $user;
            }
            

    if ($validUser != true) $errorText = "Invalid username or password!";
    
    if ($validUser == true) $_SESSION['validUser'] = true;
    else $_SESSION['validUser'] = false;
	
	return $errorText;	
}

feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]

Posted: Mon Jun 11, 2007 7:18 pm
by bdlang
So basically all they have to have is a spoofed session var of 'validUser' with a valid of TRUE and they're in.

I can only speak for myself of course, but I would check the user via session data -> database every page request. Once they're logged in, use a second table (let's call it `logins`) and record their login time, IP, user ID value, (whatever else you find useful) and a unique token that identifies this session (not the SID). Check on these values and update the time. If you want to store a 'remember me' cookie, check for this data in the cookie first, then the session, then redirect to login page.

Your login script seems to check a clear text password? Don't you store the password as a hashed value? Use at least SHA1 and a salt.

Posted: Mon Jun 11, 2007 7:55 pm
by feyd
.. or SHA256 ;)

Sorry, shameless plug.

Posted: Mon Jun 11, 2007 10:16 pm
by bdlang
feyd wrote:.. or SHA256 ;)

Sorry, shameless plug.
:D Hey, I said at least SHA1. I use your class.

Posted: Tue Jun 12, 2007 3:14 am
by Rovas
bdlang wrote: I can only speak for myself of course, but I would check the user via session data -> database every page request. Once they're logged in, use a second table (let's call it `logins`) and record their login time, IP, user ID value, (whatever else you find useful) and a unique token that identifies this session (not the SID). Check on these values and update the time. If you want to store a 'remember me' cookie, check for this data in the cookie first, then the session, then redirect to login page.

Your login script seems to check a clear text password? Don't you store the password as a hashed value? Use at least SHA1 and a salt.
I think you gone a little overboard checking IP: it can change especially if the user is in a DHCP network or where it logged from (country, city). Some may think as invasion of privacy.
Use the user ID value from the table, the time when he logged in (to use expiration session) and put the things you settle on in the privacy agreement so don' t have any problems later.

Posted: Tue Jun 12, 2007 4:15 am
by patrikG
Quick drive-by comment:

Code: Select all

die('Could not connect: ' . mysql_error())
is great for development, but not great when the site is live. People can learn from that where your classes live and all sorts of details that should be no-ones business.
Once the site is live, have the error logged in a log-file (and if you want to be extremely thorough) email the admin with the error & session details) and redirect the visitor to a generic page a la "database problem, please try again".

Posted: Tue Jun 12, 2007 8:38 am
by bdlang
Rovas wrote:I think you gone a little overboard checking IP: it can change especially if the user is in a DHCP network or where it logged from (country, city). Some may think as invasion of privacy.
Use the user ID value from the table, the time when he logged in (to use expiration session) and put the things you settle on in the privacy agreement so don' t have any problems later.
You're right of course. I don't check the IP, I just store it with the user's login data, out of habit I suppose.

Posted: Thu Jun 21, 2007 10:16 am
by big_boss
Thanks for your replies.

Posted: Thu Jun 21, 2007 10:22 am
by big_boss
bdlang wrote:So basically all they have to have is a spoofed session var of 'validUser' with a valid of TRUE and they're in.
General question: how can they spoof sessions variables when the code is executed on the web server?

Posted: Thu Jun 21, 2007 10:34 am
by The Phoenix
big_boss wrote:General question: how can they spoof sessions variables when the code is executed on the web server?
Take a read about sessions.. lots of misconceptions about how they work (from a security point of view)

http://phpsec.org/projects/guide/4.html (make sure to click through to the other pages as well)