Is my login Secure?

Talon

I have been needing to know if the login I have created is secure, since I will be using it on my admin panel. Can someone please help me?

<?php

include( "config.php" );

if( isset( $_POST['username'] ) && isset( $_POST['password'] ) )
{
    setcookie( "username", addslashes( $_POST['username'] ) );
    setcookie( "password", addslashes( $_POST['password'] ) );
    header( "Location: " . $_SERVER['PHP_SELF'] );
}
else
{
    if( isset( $_COOKIE['username'] ) && isset( $_COOKIE['password'] ) 

)
    {
        $query = "SELECT * FROM `users` WHERE Uname = '" . 

$_COOKIE['username'] . "' AND Rpass = '" . $_COOKIE['password'] . "'";
        $res = mysql_query( $query ) or die( "Query: " . $query . " 

failed." );
        $rows = mysql_num_rows( $res );
        if( !$rows )
        {
            die("Username or password invalid!");
        }
        if( isset( $_POST['update'] ) )
        {
            $pass = addslashes( $_POST['new_pass'] );
            $update = "UPDATE members SET password = '" . $pass . "' 

WHERE username = '" . $_COOKIE['username'];
            mysql_query( $update ) or die( "Unable to update your 

password, please try again!" );
        }
        else
        {
            $row = mysql_fetch_row( $res );
            ?>
            <center>
            <font size="5">Logged in as: <?= htmlentities( $row[0] ); 

?></font><br /><br />
            <b>::Change Your Password::</b>
            <form action="<?= htmlentities( $_SERVER['PHP_SELF'] ); ?>" 

method="POST"><br />
            New Password: <input type="text" name="new_pass" /><br />
            <input type="submit" name="submit" value="::Change Pass::" 

/>
            </form>
            </center>
            <?php
        }
    }
    else
    {
        ?>
        <center>
        <b>::Log In::</b>
        <form action="<?= htmlentities( $_SERVER['PHP_SELF'] ); ?>" 

method="POST"><br />
        Username: <input type="text" name="username" /><br />
        Password: <input type="password" name="password" /><br />
        <input type="submit" name="submit" value="::Log In::" />
        </form>
        </center>
        <?php
    }
}

?>

sneakyimp

You are not escaping or screening the cookie username or password before inserting their values directly into a query. This could cause broken syntax if the username or password contains a single quote ('). Worse, a smart hacker could do some SQL injection to guarantee that your query always returns a record.

You are infact using addslashes when u set the cookie, but there is nothing to stop the user from altering those cookies to be whatever they want. try removing those addslashes calls and then change your query to something like this:

$query = "SELECT * FROM `users` WHERE Uname='" . mysql_real_escape_string($_COOKIE['username']) . "' AND Rpass='" . mysql_real_escape_string($_COOKIE['password']) . "'";

Another potential problem is that your script doesn't check to make sure that any username or password is entered. I'm not sure how thorough you've been with the rest of your code, but it is potentially possible that empty rows could find their way into your table in which case leaving user and pass blank would let someone login by simply leaving those blank. It's not uncommon for garbage or empty data to find its way into a database. I would make sure strlen() for user and pass is >= 1

NogDog

Are you connecting via SSL (https://...)? If not, network sniffers could possibly pick up your user login and password on every page access, as you're passing them across the internet via the cookies.

Talon

Thanks mate, I was thinking that SQL could be put into the cookies, but I needed to make sure that I wasn't messing up on anything else [which it appears I did].

Squirreliness

Hmm...It looks fairly secure to me, other than what the above people commented on.

Piranha

You insert the password directly into the database. That is not a secure way of storing a password, you would be better off by storing a password that have been hashed with for example [man]md5[/man].

Sneakyimp suggests that you should set strlen >= 1. I suggest that you decide how many characters you should allow as a minimum and then set the strlen to at least that number. In a password I would recommend you to use at least 5 characters, more is better.

You currently check that some row have been returned before allowing access. I would check that exactly one row is returned. It may be paranoia, but better be to safe.

Weedpacket

In fact, why return an entire row? Why not just return a count of how many rows would have been returned if you'd asked for them? Smaller and more convenient (if the query isn't broken, you get guaranteed one record with one field).

Select COUNT(*) from ......