Этот код безопасен?

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, '/'); здесь?

Вот ошибка: Я использую эту форму авторизации:

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

Когда вы выходите из системы, сначала вы ставите в очередь уничтожение куки (это произойдет после отправки ответа), а затем сразу же после рендеринга страницы. Браузер не имеет возможности удалить куки перед рендерингом, а ваши $_SESSION переменные еще живы.

PHP документы говорят о session_destroy:

session_destroy () уничтожает все данные, связанные с текущим сеансом. Он не сбрасывает глобальные переменные, связанные с сеансом, и не сбрасывает cookie сеанса.

Решение состоит в том, чтобы вместо уничтожения сеанса и файла cookie просто сбросить переменные, которые могут вызвать аутентификацию:

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

Просто примечание: я бы предложил разбить ваш код на функции. Это сделало бы его намного более организованным и читаемым (и многократно используемым, если вы все делаете правильно).

Answer 2

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

Answer 3

Некоторые замечания по безопасности:

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

лучше записать как

if(!$matchingUser['inactive'])

Потому что, если схема базы данных изменится (например, теперь это целое число для обозначения определенного типа действия (что, на мой взгляд, плохой дизайн: перечисление будет лучше)), ваш код будет иметь проблемы.

Конечно, это двойной негатив, который может быть менее читабельным. Лучше было бы:

if($matchingUser['isactive'])

Или даже:

if($matchingUser->isActive())

при условии, что вы создаете класс User и т. Д. И т. Д.

На всякий случай используйте или require или require_once, где это необходимо (желательно последнее, если connect.php содержит объявления функций).

Сохранить идентификатор пользователя в переменной сеанса вместо имени пользователя. Существует возможность, что вы позволите пользователю изменить свое имя позже, и данные сеанса будут недействительными (по крайней мере, ['user'] будет, в любом случае). Также быстрее найти запись в базе данных по идентификатору (первичный ключ, уникальный), чем по имени пользователя (может быть, индексировано, строка).

Пинать меня через 30 минут действительно раздражает. Вы не единственный сайт, на который я захожу, и я могу вернуться к нему позже, выполнив некоторую работу (например, если мне позвонят, чтобы что-то сделать, или взять обеденный перерыв).

Используйте htmlspecialchars, чтобы предотвратить XSS.

Не нужно использовать $_SERVER['PHP_SELF'] здесь:

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

Просто напишите без него:

<a href="?logout=true">

Когда пользователь что-то публикует, обязательно перенаправьте его (TODO: обратите внимание, как). В противном случае кнопка «Назад» пользователя может привести к повторной отправке данных (что, вероятно, не то, что вам нужно!).

Answer 4

MD5 - довольно слабое средство для шифрования пароля, и существует множество способов его обойти. Однако это помогает, если вы настроили IP-адрес, но IP-адреса, безусловно, немного изменились.

Кроме того, у вас нет ничего, чтобы убедиться, что кто-то не взломал систему снова и снова, чтобы взломать пароль для пользователя.

G-Man

Answer 5

Используйте соль вместе с MD5 или sha1. Что я использую для генерации пароля и проверки пароля при входе в систему.

function generateHash($plainText, $salt = null)
{
    define('SALT_LENGTH', 9);
    if ($salt === null)
    {
        $salt = substr(md5(uniqid(rand(), true)), 0, SALT_LENGTH);
    return array($salt, sha1($salt . $plainText) );
    }
    else
    {
        $salt = substr($salt, 0, SALT_LENGTH);
    return sha1($salt . $plainText);
    }

}

Для $ plainText отправьте переменную пароля.

Если вы хотите сгенерировать новый хеш, он вернет 2 значения. Первое значение называется «соль», а второе значение является зашифрованным паролем. Сохраните их в базе данных.

Когда кто-то пытается войти на ваш сайт, и вы хотите проверить, отправьте свой пароль и переменную соли функции, и она вернет хеш. Затем вы можете сравнить это со значением, хранящимся в базе данных, чтобы проверить правильность введенного пользователем пароля.

Answer 6

$_SERVER['REMOTE_ADDR'] может измениться, если пользователь находится за прокси.

Answer 7

Вы можете использовать этот метод, описанный Нейтом Абелем , для повышения надежности информации, используемой для создания отпечатка пальца.

Answer 8

В этом случае вы должны убедиться в правильности настроек вашего сервера для правильной работы mysql_real_escape_string. В этом случае ваш код предполагает, что Magic Quotes отключены, в то время как я часто нахожу, что на многих серверах он включен. Вам, вероятно, следует проверить, возвращает ли get_magic_quotes_gpc() значение true или false, чтобы убедиться, что настройки сервера автоматически экранируют строки.

Код выглядит безопасным, но я бы посоветовал изучить новый способ выполнения запросов MySQL: PDO . Это учитывает параметризованные запросы.

Answer 9

Что означает setcookie (session_name (), '', time () - 3600, '/');

Похоже, что он стирает старый файл cookie сеанса, устанавливая для него пустую строку и устанавливая время истечения в прошлом. Не уверен, почему автор использовал оба метода.

Answer 10

выглядит хорошо. setcookie(session_name(), '', time()-3600, '/') по существу удаляет cookie, устанавливая время до текущего времени.