这段代码安全吗？

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;
}
?>

这只是http://en.wikibooks.org/wiki/PHP_Programming/User_login_systems的修改版本

setcookie(session_name(), '', time()-3600, '/');在这里做什么？

这里有一个bug:我使用这个登录表单：

<?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>';
    }
?>

错误是，当我注销时，页面保持不变，即登录表单不显示，但显示相同的注销和用户。当我刷新页面时，它变得正常。

Answer 1

当您注销时，首先，您正在排队销毁cookie (将在发送响应之后发生)，然后立即呈现您的页面。浏览器没有机会在呈现之前删除cookie，并且您的$_SESSION变量仍然有效。

PHP文档提到了session_destroy

session_destroy()销毁与当前会话相关的所有数据。它不会取消设置与会话关联的任何全局变量，也不会取消设置会话cookie。

一种解决方案是，不破坏会话和cookie，只需取消设置会导致身份验证的变量：

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

只需注意:我建议将您的代码拆分成函数。这将使它更有条理，更具可读性(如果你做对了，还可以重用)。

Answer 2

一些安全说明：

if($matchingUser['inactive'] == 1)

最好写成

if(!$matchingUser['inactive'])

因为如果数据库模式发生变化(例如，它现在是一个整数，表示某种类型的活动(在我看来，这是一个糟糕的设计:枚举会更好))，您的代码将会出现问题。

当然，这是一个双重否定，可读性可能较差。更好的方法是：

if($matchingUser['isactive'])

或者甚至是：

if($matchingUser->isActive())

假设你创建了一个用户类，等等。

为了以防万一，在必要时使用或require或require_once (如果connect.php包含函数声明，最好使用后者)。

将用户ID存储在会话变量中，而不是用户名中。您可能会允许用户稍后更改其姓名，而会话数据将是无效的(至少['user']会这样做)。按ID (主键，唯一)查找数据库记录也比按用户名(可能是索引，字符串)更快。

30分钟后踢我真的很烦人。你不是我唯一去过的网站，我可能会在做完一些工作后再去访问(例如，如果我被叫去做某事，或者午休)。

使用htmlspecialchars来帮助防止XSS。

这里不需要使用$_SERVER['PHP_SELF']：

<a href="' . $_SERVER["PHP_SELF"] . '?logout=true">

只需不使用它即可编写：

<a href="?logout=true">

当用户发布内容时，请确保将其重定向(TODO:注意如何发布)。否则，用户的back按钮可能会导致重新发布数据(这可能不是您想要的！)。

Answer 3

永远不会设置$matchingUser['inactive']，因为您无法使用mysql_fetch_assoc()从数据库中获取实际数据。

修改版本：

$matchingUser = mysql_query("SELECT * FROM `users` WHERE username='$user' AND password=MD5('$password') LIMIT 1");
if (mysql_num_rows($matchingUser))
{
    $matchingUserData = mysql_fetch_assoc($matchingUser);
    if($matchingUserData['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;
    }