PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Mon Nov 18, 2019 7:45 am

All times are UTC - 5 hours




Post new topic Reply to topic  [ 9 posts ] 
Author Message
PostPosted: Tue Nov 08, 2011 11:40 am 
Offline
Forum Newbie

Joined: Tue Nov 08, 2011 11:32 am
Posts: 5


Top
 Profile  
 
PostPosted: Tue Nov 08, 2011 2:18 pm 
Offline
Moderator
User avatar

Joined: Tue Nov 09, 2010 3:39 pm
Posts: 6425
Location: Montreal, Canada
You're right that storing the password in plain text in the database is a bad idea. Storing an md5 hash is definitely better, but still not particularly secure. I'd recommend adding salt and pepper and using a stronger hashing algorithm. Use a common salt for all your users (just a string you store somewhere) and a pepper that's unique to each user (their user ID, account creation time, etc), concatenate the whole mess and hash the result.

_________________


Top
 Profile  
 
PostPosted: Tue Nov 08, 2011 2:46 pm 
Offline
Forum Regular
User avatar

Joined: Wed Mar 05, 2008 11:23 pm
Posts: 732
Location: Sunriver, OR
It looks like you trust your users to provide correct information.

How does your code handle incorrect credentials?
How does your code handle incomplete credentials?
How does your code handle specials characters?

Syntax: [ Download ] [ Hide ]
/* As a user, in your form, I type the following info:
 username = *
 password = ' OR 1='1

  # My point is, you need to learn about SQL Injection
*/



$myusername = $_POST['myusername'];
$mypassword = $_POST['mypassword'];

$sql = "SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";


Top
 Profile  
 
PostPosted: Tue Nov 08, 2011 3:55 pm 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
session_register() was an old technique even back in 2002 and it amuses me to no end to still see it in "tutorials", especialy "security" "tutorials" (even the quotes should have quotes :) )

Apart from the sql injection and cleartext passwords already pointed out, you should care for session fixation (use session_regenerate_id after login) and the login checking code -> call exit() after header('Location...'), otherwise any code below will still be executed.


Top
 Profile  
 
PostPosted: Tue Nov 08, 2011 4:56 pm 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2794
Location: .za
When using mysql_connect() an option is to use the error control operator (@) in front of the function to stop any sensitive information from being displayed in case the database connection cannot be made. Same goes for the mysql_select_db() statement and the die(mysql_error()) at the end of that statement; it's good to have it in production but if your site is live, remove it or deal with the resulting error in another, "clean" way.

Another point, maybe not directly related to security but more an efficiency point is to use sql's COUNT() function to find a matching row within the database. It involves different code in retrieving the amount of rows returned but you don't have to use mysql_num_rows().
Syntax: [ Download ] [ Hide ]
<?php
 $sql = "SELECT COUNT(id) FROM usertbl WHERE password = '$password' AND username = '$username' ";
?>


Don't use short tags ( <? ?> ).

_________________
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering


Top
 Profile  
 
PostPosted: Tue Nov 08, 2011 5:39 pm 
Offline
Forum Newbie

Joined: Tue Nov 08, 2011 11:32 am
Posts: 5


Top
 Profile  
 
PostPosted: Tue Nov 08, 2011 6:06 pm 
Offline
Forum Regular
User avatar

Joined: Wed Mar 05, 2008 11:23 pm
Posts: 732
Location: Sunriver, OR


Top
 Profile  
 
PostPosted: Wed Nov 09, 2011 12:39 am 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2794
Location: .za

_________________
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering


Top
 Profile  
 
PostPosted: Tue Nov 15, 2011 6:11 pm 
Offline
Forum Newbie

Joined: Tue Nov 08, 2011 11:32 am
Posts: 5


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 9 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 4 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB® Forum Software © phpBB Group