basic security

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

Post Reply
big_boss
Forum Newbie
Posts: 3
Joined: Mon Apr 02, 2007 8:50 am

basic security

Post 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]
bdlang
Forum Contributor
Posts: 395
Joined: Tue May 16, 2006 8:46 pm
Location: Ventura, CA US

Post 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.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

.. or SHA256 ;)

Sorry, shameless plug.
bdlang
Forum Contributor
Posts: 395
Joined: Tue May 16, 2006 8:46 pm
Location: Ventura, CA US

Post by bdlang »

feyd wrote:.. or SHA256 ;)

Sorry, shameless plug.
:D Hey, I said at least SHA1. I use your class.
Rovas
Forum Contributor
Posts: 272
Joined: Mon Aug 21, 2006 7:09 am
Location: Romania

Post 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.
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post 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".
bdlang
Forum Contributor
Posts: 395
Joined: Tue May 16, 2006 8:46 pm
Location: Ventura, CA US

Post 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.
big_boss
Forum Newbie
Posts: 3
Joined: Mon Apr 02, 2007 8:50 am

Post by big_boss »

Thanks for your replies.
big_boss
Forum Newbie
Posts: 3
Joined: Mon Apr 02, 2007 8:50 am

Post 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?
User avatar
The Phoenix
Forum Contributor
Posts: 294
Joined: Fri Oct 06, 2006 8:12 pm

Post 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)
Post Reply