Is this code secure?

Question

<?php
session_start();

include("connect.php");

$timeout = 60 * 30;
$fingerprint = md5($_SERVER['REMOTE_ADDR'] . $_SERVER['HTTP_USER_AGENT']);

if(isset($_POST['userName']))
{
    $user = mysql_real_escape_string($_POST['userName']);
    $password = mysql_real_escape_string($_POST['password']);
    $matchingUser = mysql_query("SELECT * FROM `users` WHERE username='$user' AND password=MD5('$password') LIMIT 1");
    if (mysql_num_rows($matchingUser))
    {
        if($matchingUser['inactive'] == 1)//Checks if the inactive field of the user is set to one
        {
            $error = "Your e-mail Id has not been verified. Check your mail to verify your e-mail Id. However you'll be logged in to site with less privileges.";
            $_SESSION['inactive'] = true;
        }
        $_SESSION['user'] = $user;
        $_SESSION['lastActive'] = time();
        $_SESSION['fingerprint'] = $fingerprint;
    }
    else
    {
        $error = "Invalid user id";
    }
}
if ((isset($_SESSION['lastActive']) && $_SESSION['lastActive']<(time()-$timeout)) || (isset($_SESSION['fingerprint']) && $_SESSION['fingerprint']!=$fingerprint)
     || isset($_GET['logout'])
    )
{
    setcookie(session_name(), '', time()-3600, '/');
    session_destroy();
}
else
{
    session_regenerate_id(); 
    $_SESSION['lastActive'] = time();
    $_SESSION['fingerprint'] = $fingerprint;
}
?>

This is just modified version of http://en.wikibooks.org/wiki/PHP_Programming/User_login_systems

What does the setcookie(session_name(), '', time()-3600, '/'); do here?

Here's a bug: I use this login form:

<?php 
   if(!isset($_SESSION['user']))
    {
        if(isset($error)) echo $error;
           echo '<form action="' . $_SERVER["PHP_SELF"] . '" method="post">
        <label>Username: </label>
        <input type="text" name="userName" value="';if(isset($_POST['userName'])) echo $_POST["userName"]; echo '" /><br />
        <label>Password: </label>
        <input type="password" name="password" />
        <input type="submit" value="Login" class="button" />
        <ul class="sidemenu">
        <li><a href="register.php">Register</a></li>
        <li><a href="forgotPassword.php">Forgot Password</a></li>
    </ul>
    </form>';
    }
    else
    {
        echo '<ul class="sidemenu">
        <li>' . $_SESSION['user'] . '</li>
        <li><a href="' . $_SERVER["PHP_SELF"] . '?logout=true">Logout</a></li>
        </ul>';
    }
?>

The bug is that when I logout, the page remains the same, i.e. the login form does not show up but the same logout and user are shown. When I refresh the page, it gets normal.

Answer 1

When you log out, first, you are queuing the destruction of the cookie (it will occur after the response is sent), then immediately after, rendering your page. The browser has no chance to delete the cookie before rendering, and your $_SESSION variables are still alive.

PHP docs say about session_destroy:

session_destroy() destroys all of the data associated with the current session. It does not unset any of the global variables associated with the session, or unset the session cookie.

A solution is to, instead of destroying the session and the cookie, simply unset the variables which would cause authentication:

unset($_SESSION['user']);
unset($_SESSION['lastActive']);
unset($_SESSION['fingerprint']);

Just a note: I would suggest splitting your code up into functions. This would make it much more organized and readable (and reusable if you do things right).