is this secure login script??

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
User avatar
PHPycho
Forum Contributor
Posts: 336
Joined: Fri Jan 06, 2006 12:37 pm

is this secure login script??

Post by PHPycho »

I am using the following code for user login ..

PHP Code:

Code: Select all

<?php 
session_start(); 
?> 
<?php 
if(isset($_POST['nposted'])) 
         { 
if($_POST['nname']=="" or $_POST['npass']=="") 
{ 
 echo "<center><img src=\"./images/error.gif\"> 
 <font color='red'><b>Error!</b><br>Enter all the fields:</font><br>"; 
 echo "<br> 
 Click <a href='$_SERVER[PHP_SELF]'><b>[here]</b></a> to Go Back</center>"; 
} 
else 
   { 
$nname=$_POST['nname']; 
$npass=$_POST['npass']; 
include "db_config.php"; 
@mysql_connect($host,$user,$pass) 
or die(mysql_error()); 
@mysql_select_db($db) 
or die(mysql_error()); 
$sql="SELECT * from user where user='$nname' AND pass='$npass'"; 
$result=mysql_query($sql); 
$num=mysql_num_rows($result); 

if($num!=0) 
     { 
 $_SESSION['nadmin']=$nname; 
 $_SESSION['npass']=$npass; 
 echo"<br> 
<center><b>User page is Loading................Plz Wait.</b></center>"; 
 echo "<META HTTP-EQUIV=Refresh CONTENT=\"0; URL=userpage.php\"> "; 
     } 
else 
{ 
 echo "<center><img src=\"./images/error.gif\"> 
 <font color='red'><b>Error!</b><br>Sorry ID and Password didn't match:</font><br>"; 
 echo "<br> 
 Click <a href='$_SERVER[PHP_SELF]'><b>[here]</b></a> to Go Back</center>"; 
} 
         } 
} 
else 
 { 
 echo " 
 <form action='$_SERVER[PHP_SELF]' method='post'> 
<input type='hidden' name='nposted' value='true'> 
username<input type='text' name='nname'> 
password<input type='password' name='npass'> 
<input type='submit' value='login'> 
</form>"; 
 } 

?>
Is the above login page is secure ??
If not what are the modifiaction required...Help me with the illustrated code...
Thank u in advance..
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

Post by hawleyjr »

User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Well, there are a few problems. It all stems from not applying the "filter and escape user input" rule.

You have to keep in mind that a user can pass ANY data to ANY variable through GET and POST - so you need to make absolutely certain you get exactly what is expected.

Take $_POST['nname'].

How do you know the user didn't pass some url-encoded SQL to insert extra SQL statements? Same goes for the password variable.

The way you should think is, what is nname? Alphabetic? Alphanumeric? Does it allow underscores and/or spaces? Any other special characters?

If it was say alphanumeric only, you should filter the data by for example...

Code: Select all

$clean = array();

$clean['nname'] = '';
if(isset($_POST['nname']) && ctype_alnum($_POST['nname']) && strlen($_POST['nname']) <= 32) {
    // nname is now verified as set, an alphanumeric string, with a 32 maximum character length (to match maxlength attribute of form field in html.
    $clean['nname'] = $_POST['nname'];
}
You now know that nname is definitely alphanumeric - no other funny characters, or xss strings, or sql strings...;)

Next when entering it into the database we also assume our filtering failed. This will strike you as a very odd way of thinking, but blunders happen on the smallest of human errors - so we play it safe and make a second protective act - we escape the data for the database.

So for mysql:

Code: Select all

$sql = array();

$sql['nname'] = mysql_real_escape_string($clean['nname']);
$sql="SELECT * from user where user='" . $sql['nname'] . "'";
The exact same procedure should apply for passwords - and all other user data (basically the entire contents of GET and POST!).

Something else is $_SERVER['PHP_SELF']. This also needs to be filtered - the value can be set on some server settings by the user. As a result a user could get other users to visit a tainted url which inserts arbitrary XSS. Without filtering this could make your site an unwitting accomplice!

So either filter the PHP_SELF variable - or explicitly set a fixed action attribute.

Just one curious question - where is the user saved to the database for future logins?
User avatar
PHPycho
Forum Contributor
Posts: 336
Joined: Fri Jan 06, 2006 12:37 pm

Post by PHPycho »

Thanks alot for gr8 suggestions.......
again i want to interrupt u.....
is storing the password in session as

Code: Select all

$_SESSION['npass']=$npass;

secure??
if not what are the required steps??
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

Why would you need to store the password in a session variable?
Why do you store plaintext password in the database?
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

Post by hawleyjr »

Here is a basic example of storing a password with salt into a db.

viewtopic.php?t=40847#217197
User avatar
PHPycho
Forum Contributor
Posts: 336
Joined: Fri Jan 06, 2006 12:37 pm

Post by PHPycho »

I am storing the password in SESSION to check the previlage of the user .....if the SESSION variable with password is not set then the page redirect to the login page....
Or is there any way to secure the restricted page without using session and cookies??
If yes then Please suggest me
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

Post by hawleyjr »

I do not recomend storing the password. Set a flag instead.

Code: Select all

if($isvaliduser == TRUE)
$_SESSION['VALID_USER'] = TRUE;
else
$_SESSION['VALID_USER'] = FALSE;


///And then on every page just check for the session var

if($_SESSION['VALID_USER'] ===TRUE)
//valid user
else
//not valid user
mickd
Forum Contributor
Posts: 397
Joined: Tue Jun 21, 2005 9:05 am
Location: Australia

Post by mickd »

if you are storing the password to check if the user is valid, what would happen if 2 users both share the same password?

storing their username/user id and if you want, their access level is usually enough depending on your database structure.
StumpDK
Forum Commoner
Posts: 35
Joined: Thu Feb 12, 2004 2:28 am
Location: Copenhagen, Denmark

Post by StumpDK »

I do not recomend storing the password. Set a flag instead.

Code: Select all

if($isvaliduser == TRUE) 
$_SESSION['VALID_USER'] = TRUE; 
else 
$_SESSION['VALID_USER'] = FALSE; 


///And then on every page just check for the session var 

if($_SESSION['VALID_USER'] ===TRUE) 
//valid user 
else 
//not valid user
But isn't that very unsecure? What if a hacker on another computer makes a session-variable named VALID_USER with the value TRUE? In that case, he will be able to access the site when he wants.
Or am I wrong?
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

StumpDK wrote: But isn't that very unsecure? What if a hacker on another computer makes a session-variable named VALID_USER with the value TRUE? In that case, he will be able to access the site when he wants.
Or am I wrong?
And how would he make that session variable?? (He would need access to your webserver. As soon as he gets that why wouldn't he immediately get the required data instead of messing around with your website?)
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Thing you have to keep in mind is that PHP reads and writes the session on the local server - its not stored, nor can it be overwritten by any client, browser or user.

The only portion of a session a hacker can change is their session_id - this is a cookie value that identifies your web requests with any session data PHP has stored. They cannot change the actual session data though.
Post Reply