If this is your first visit, be sure to check out the FAQ by clicking the link above. You may have to register before you can post: click the register link above to proceed. To start viewing messages, select the forum that you want to visit from the selection below.

 
Go Back  dBforums > Data Access, Manipulation & Batch Languages > PHP > login script

Reply
 
LinkBack Thread Tools Search this Thread Display Modes
  #1 (permalink)  
Old 01-05-12, 09:33
dbecker88 dbecker88 is offline
Registered User
 
Join Date: Nov 2011
Posts: 16
login script

hey guys -

would someone mind looking over my login script for any huge security issues?

this is actually a shortcode in a wodpress site...hence the html form in here also. After successful auth. the user is redirected to employees.domain.com, and the session_id is stored in the URL.

Code:
if (isset($_GET['sid']) && trim($_GET['sid']) != '') {
	session_id($_GET['sid']);
}
session_start();
date_default_timezone_set('America/city');
$date = date('l jS F Y h:i A T');
$hostname = 'localhost';
$dbname   = 'mydb';
$username = 'mysql_un';
$password = 'mysql_pw';
mysql_connect($hostname, $username, $password) or DIE('Connection to host is failed, perhaps the service is down!');
mysql_select_db($dbname) or DIE('Database name is not available!');

if(isset($_POST['username'])){
	if(isset($_POST['password'])){

		$login = mysql_query("SELECT * FROM users WHERE (username = '" . mysql_real_escape_string($_POST['username']) . "') and (password = '" . mysql_real_escape_string(md5($_POST['password'])) . "')");
		$num_results = mysql_num_rows($login);


		if ($num_results == 1){
			$_SESSION['username'] = $_POST['username'];
			$_SESSION['password'] = $_POST['password'];
			while ($row = mysql_fetch_assoc($login)){
				$_SESSION['priv'] = $row['priv'];
				$_SESSION['email'] = $row['email'];
			}
			$tbl_name2="access_log";
			$user = $_POST['username'];
			$addrecord = mysql_query("INSERT INTO $tbl_name2 (id, username, date) VALUES ('','$user','$date')");
			if(isset($_SESSION['redir_after_auth'])){
				
				header('Location: http://employees.domain.com' . $_SESSION['redir_after_auth']);
				die();
			}
			else {
				header('Location: http://employees.domain.com?sid=' . session_id());
				unset($_SESSION['$num_results']);
				die();
			}
		}
		else {
			?>
			<table align="center">
			<tr><td align="center">
			<p style="color:#F00">
			<?php echo 'Incorrect Username or Password'; ?>
			</p>
			</td></tr>
			</table>
			<?php
		}
	}
}
?>


<div>
<form action="" method="POST">
<table align="center">
<tr><td align="left">Username: </td><td align="left"><input type="text" name="username"></td></tr>
<tr><td align="left">Password: </td><td align="left"><input type="password" name="password"></td></tr>
<tr><td align="left"><input type="submit" value="Login"></td></tr>
</table>
</form>
</div>

here is the landing page of employees.domain.com. The URL is checked for session_id, and if found, the session is started and page is loaded.

Code:
<?php
if (isset($_GET['sid']) && trim($_GET['sid']) != '') {
	session_id($_GET['sid']);
	}
session_start();
// Check, if user is already login, then load contents of page page
if (isset($_SESSION['username'])) 
{
?>
html page content here
<?php
}
else
{
	$_SESSION['redir_after_auth'] = $_SERVER['PHP_SELF'];
	header('Location: http://login.domain.com?page_id=258&sid='.session_id());
}
?>
Reply With Quote
  #2 (permalink)  
Old 01-06-12, 09:40
dbecker88 dbecker88 is offline
Registered User
 
Join Date: Nov 2011
Posts: 16
no suggestions?
Reply With Quote
  #3 (permalink)  
Old 01-06-12, 21:52
gvee gvee is offline
www.gvee.co.uk
 
Join Date: Jan 2007
Location: UK
Posts: 10,156
Couple of quick pointers:

Don't use "SELECT * " - list out only the columns you require.

What is the reason for putting the session_id in the querystring? Why not store it in a session variable instead i.e. somewhere the user can't see.

In fact, why store it at all? If you need it at all you can just grab is using the session_id() function.

So instead use session variables to store something to show the user is authenticated or not. PHP Sessions

Another thing I spotted is this line:
Code:
$user = $_POST['username'];
I think it would be best to populate your $user variable from your initial SELECT statement.

Also, why have you escaped your variables in the first SQL statement and not the second? Any reason not to do it in the second?

Hope this helps
__________________
George
Twitter | Blog
Reply With Quote
  #4 (permalink)  
Old 01-06-12, 23:06
dbecker88 dbecker88 is offline
Registered User
 
Join Date: Nov 2011
Posts: 16
Quote:
Originally Posted by gvee View Post
Couple of quick pointers:

Don't use "SELECT * " - list out only the columns you require.

What is the reason for putting the session_id in the querystring? Why not store it in a session variable instead i.e. somewhere the user can't see.

In fact, why store it at all? If you need it at all you can just grab is using the session_id() function.

So instead use session variables to store something to show the user is authenticated or not. PHP Sessions

Another thing I spotted is this line:
Code:
$user = $_POST['username'];
I think it would be best to populate your $user variable from your initial SELECT statement.

Also, why have you escaped your variables in the first SQL statement and not the second? Any reason not to do it in the second?

Hope this helps
thanks for the reply.
I put the session_id in the URL simply because I didn't have access to $_SESSION variables set in employees.domain.com inside of domain.com. I *thought* this was the only way to 'carry' $_SESSION variables across domains...even though the sites are on the same box?? Please correct me if i'm wrong.

after the session_id is restored using $_GET at domain.com, the $_SESSION variables that were set at employees.domain.com are then accessible.

I'll clean up the other tid-bits...thanks again!!
Reply With Quote
  #5 (permalink)  
Old 01-07-12, 04:34
healdem healdem is offline
Jaded Developer
 
Join Date: Nov 2004
Location: out on a limb
Posts: 9,262
not too certain how session variables are treated, but I thought UK sites now had to request the users consent to set / use cookies
__________________
I'd rather be riding my Versys or my Tiger 800 let alone the Norton
Reply With Quote
  #6 (permalink)  
Old 01-07-12, 16:20
kitaman kitaman is offline
Papabi's friend
 
Join Date: Sep 2009
Location: Ontario
Posts: 629
Add a loop in the invalid userid password paragraph to track the remote ip address, and then ban all attempts from that ip after 4 (or 5?) consecutive failed attempts.
Reply With Quote
  #7 (permalink)  
Old 01-14-12, 13:18
dbecker88 dbecker88 is offline
Registered User
 
Join Date: Nov 2011
Posts: 16
Quote:
Originally Posted by kitaman View Post
Add a loop in the invalid userid password paragraph to track the remote ip address, and then ban all attempts from that ip after 4 (or 5?) consecutive failed attempts.
write the remote ip to mysql, then upon login, query the mysql ip table and if ip listed, jump to failed login?
Reply With Quote
  #8 (permalink)  
Old 01-14-12, 19:12
kitaman kitaman is offline
Papabi's friend
 
Join Date: Sep 2009
Location: Ontario
Posts: 629
Yes,
Create a table with at least
IPaddress, failed_login_count
You could add date, time, userid_used, password_used

Look up IPaddress, and if login credentials are invalid, add 1 to failed_login_count.
If second or third attempt is correct, reset failed_login_count to zero.
If failed_login_account exceeds 4?5? refuse login even if credentials are correct.
Send email to system administrator, and to account holder if the failed user id is valid but the password is not.
Reply With Quote
Reply

Thread Tools Search this Thread
Search this Thread:

Advanced Search
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are On