views:

94

answers:

1

I just wanted to run my login script by you guys to see if there are other security measures that i should take.

I know that i need to move my DB constants to a config file but aside from that and some error handling, what other checks to i need to add to make sure this script is secure?

<?php
    ob_start();
    session_start();
    require 'phpDatabaseClass.php';

    define('DBhost', 'localhost');
    define('DBusername', 'root');
    define('DBpassword', 'root');
    define('DBname', 'campbellCustomCoatings');


    if(isset($_POST['submit']))
    {       
        if((!empty($_POST['username'])) && (!empty($_POST['password'])))
        {
            //creates the db object and the constructor automatically creates the db connection
             $db = new phpDatabaseClass();

            //sets the username and password to variables and sanitizes them
            $username = $_POST['username'];
            $password = $_POST['password'];

            //sets the username to only alphanumeric characters
            $username = preg_replace('/[^A-Za-z0-9]/', '', $_POST['username']);

            //if the username and password are valid
            if($db->validateLogin($username, $password))
            {
                $_SESSION['loggedIn'] = $username;

                header('Location: uploadImages.php');
            }
        }        
    }
?>

and my phpDatabaseClass

<?php
/********************************************
* Created by Tom Caflisch
* 
* Class to connect and query a mysql database 
*
*********************************************/

class phpDatabaseClass {

    private $mysqli;

    /****************************
    * Constructor function which makes the connection to the database
    ****************************/
    public function __construct()
    {
        $this->mysqli = new mysqli(DBhost, DBusername, DBpassword, DBname);
    }
/****************************
    * Function which checks to see if the username and password validate
    *
    * $username is the username that the user entered in the form
    * $password is the password that the user entered in the form
    *
    ****************************/
    public function validateLogin($username, $password)
    {

        //if magic quotes are turned on, remove the slashes
        if(get_magic_quotes_gpc())
        {
            $username = stripslashes($username);
            $password = stripslashes($password);
        }

        $username = $this->mysqli->real_escape_string($username);
        $password = $this->mysqli->real_escape_string($password);

        $password = md5($password);

        //the query
        $sql = 'select * from users
                where username = \''.$username.'\'
                and password = \''.$password.'\'';

        $results = $this->mysqli->query($sql);

        //if something is wrong with the query
        if($results === false)
        {
            echo 'Whoa you trying to hack this thing?';
        }
        else 
        {
            echo $results->num_rows;
            if(($results->num_rows) == 1)
            {

                while($row = $results->fetch_row())
                {
                    //if the username and password match return true else return false
                    if($username == $row[1] && $password == $row[2])
                    {
                        return true;
                    }
                    else
                    {
                        return false;
                    }
                }   
            }
        }

        //return false;
    }


}
?>
+3  A: 

I would not use MD5 for hashing passwords, consider using something else like sha1 or whirlpool and maybe salt the passwords as well.

I find this link very useful: http://www.richardlord.net/blog/php-password-security it describes how to salt passwords and so on.

Cheesebaron
Hashing password is like thinking - ouch, my site is so insecure that everybody can read my database.
codez
@codez are you suggesting he store the passwords in plaintext? Am I to assume from your comment that you've developed the world's first 100% hacker-proof public website?
Hashing passwords is more like, "I know I did a lot of work to make sure that my login is secure for my users but I know that there are bad people out there that can and will exploit vulnerabilities that I don't know about. I'll protect my users by salting and hashing their passwords with SHA-1 just to be safe."
SHC