Results 1 to 8 of 8

Thread: login script

  1. #1
    Join Date
    Nov 2011
    Posts
    16

    Unanswered: 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());
    }
    ?>

  2. #2
    Join Date
    Nov 2011
    Posts
    16
    no suggestions?

  3. #3
    Join Date
    Jan 2007
    Location
    UK
    Posts
    11,434
    Provided Answers: 10
    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
    Home | Blog

  4. #4
    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!!

  5. #5
    Join Date
    Nov 2004
    Location
    out on a limb
    Posts
    13,692
    Provided Answers: 59
    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 on the Tiger 800 or the Norton

  6. #6
    Join Date
    Sep 2009
    Location
    Ontario
    Posts
    1,057
    Provided Answers: 1
    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.

  7. #7
    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?

  8. #8
    Join Date
    Sep 2009
    Location
    Ontario
    Posts
    1,057
    Provided Answers: 1
    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.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •