Пожалуйста, критикуйте этот скрипт входа в PHP - PullRequest
3 голосов
/ 03 января 2011

Сайт, который я разработал, недавно был взломан, вероятнее всего, из-за перебора или атаки Rainbow Table.В исходном сценарии входа в систему не было SALT, пароли были сохранены в MD5.

Ниже приведен обновленный сценарий, дополненный запретом SALT и IP-адреса.Кроме того, он отправит электронное письмо и SMS за майские дни и отключит учетную запись, если тот же IP-адрес или учетная запись попытаются выполнить 4 неудачных входа в систему.Пожалуйста, посмотрите, и дайте мне знать, что можно улучшить, чего не хватает, а что странно.

<?php
    //Start session
    session_start();
    //Include DB config
    include $_SERVER['DOCUMENT_ROOT'] . '/includes/pdo_conn.inc.php';

    //Error message array
    $errmsg_arr = array();
    $errflag = false;

    //Function to sanitize values received from the form. Prevents SQL injection
    function clean($str) {
        $str = @trim($str);
        if(get_magic_quotes_gpc()) {
            $str = stripslashes($str);
        }
        return $str;
    }

    //Define a SALT, the one here is for demo
    define('SALT', '63Yf5QNA');

    //Sanitize the POST values
    $login = clean($_POST['login']);
    $password = clean($_POST['password']);
    //Encrypt password
    $encryptedPassword = md5(SALT . $password);
    //Input Validations
    //Obtain IP address and check for past failed attempts
    $ip_address = $_SERVER['REMOTE_ADDR'];
    $checkIPBan = $db->prepare("SELECT COUNT(*) FROM ip_ban WHERE ipAddr = ? OR login = ?");
    $checkIPBan->execute(array($ip_address, $login));
    $numAttempts = $checkIPBan->fetchColumn();
    //If there are 4 failed attempts, send back to login and temporarily ban IP address
    if ($numAttempts == 1) {
        $getTotalAttempts = $db->prepare("SELECT attempts FROM ip_ban WHERE ipAddr = ? OR login = ?");
        $getTotalAttempts->execute(array($ip_address, $login));
        $totalAttempts = $getTotalAttempts->fetch();
        $totalAttempts = $totalAttempts['attempts'];
        if ($totalAttempts >= 4) {
            //Send Mayday SMS
            $to = "admin@somewhere.com";
            $subject = "Banned Account - $login";
            $mailheaders = 'From: noreply@somewhere.com' . "\r\n";
            $mailheaders .= 'Reply-To: noreply@somewhere.com' . "\r\n";
            $mailheaders .= 'MIME-Version: 1.0' . "\r\n";
            $mailheaders .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
            $msg = "<p>IP Address - " . $ip_address . ", Username - " . $login . "</p>";
            mail($to, $subject, $msg, $mailheaders);
            $setAccountBan = $db->query("UPDATE ip_ban SET isBanned = 1 WHERE ipAddr = '$ip_address'");
            $setAccountBan->execute();
            $errmsg_arr[] = 'Too Many Login Attempts';
            $errflag = true;    
        }
    }
    if($login == '') {
        $errmsg_arr[] = 'Login ID missing';
        $errflag = true;
    }
    if($password == '') {
        $errmsg_arr[] = 'Password missing';
        $errflag = true;
    }

    //If there are input validations, redirect back to the login form
    if($errflag) {
        $_SESSION['ERRMSG_ARR'] = $errmsg_arr;
        session_write_close();
        header('Location: http://somewhere.com/login.php');
        exit();
    }

    //Query database
    $loginSQL = $db->prepare("SELECT password FROM user_control WHERE username = ?");
    $loginSQL->execute(array($login));
    $loginResult = $loginSQL->fetch();

    //Compare passwords
    if($loginResult['password'] == $encryptedPassword) {
        //Login Successful
        session_regenerate_id();
        //Collect details about user and assign session details
        $getMemDetails = $db->prepare("SELECT * FROM user_control WHERE username = ?");
        $getMemDetails->execute(array($login));
        $member = $getMemDetails->fetch();
        $_SESSION['SESS_MEMBER_ID'] = $member['user_id'];
        $_SESSION['SESS_USERNAME'] = $member['username'];
        $_SESSION['SESS_FIRST_NAME'] = $member['name_f'];
        $_SESSION['SESS_LAST_NAME'] = $member['name_l'];
        $_SESSION['SESS_STATUS'] = $member['status'];
        $_SESSION['SESS_LEVEL'] = $member['level'];
        //Get Last Login
        $_SESSION['SESS_LAST_LOGIN'] = $member['lastLogin'];
        //Set Last Login info
        $updateLog = $db->prepare("UPDATE user_control SET lastLogin = DATE_ADD(NOW(), INTERVAL 1 HOUR), ip_addr = ? WHERE user_id = ?");
        $updateLog->execute(array($ip_address, $member['user_id']));
        session_write_close();
        //If there are past failed log-in attempts, delete old entries
        if ($numAttempts > 0) {
            //Past failed log-ins from this IP address. Delete old entries
            $deleteIPBan = $db->prepare("DELETE FROM ip_ban WHERE ipAddr = ?");
            $deleteIPBan->execute(array($ip_address));
        }
        if ($member['level'] != "3" || $member['status'] == "Suspended") {
            header("location: http://somewhere.com");
        } else {
            header('Location: http://somewhere.com');
        }
        exit();
    } else {
        //Login failed. Add IP address and other details to ban table
        if ($numAttempts < 1) {
        //Add a new entry to IP Ban table
        $addBanEntry = $db->prepare("INSERT INTO ip_ban (ipAddr, login, attempts) VALUES (?,?,?)");
        $addBanEntry->execute(array($ip_address, $login, 1));
        } else {
            //increment Attempts count 
            $updateBanEntry = $db->prepare("UPDATE ip_ban SET ipAddr = ?, login = ?, attempts = attempts+1 WHERE ipAddr = ? OR login = ?");
            $updateBanEntry->execute(array($ip_address, $login, $ip_address, $login));
        }
        header('Location: http://somewhere.com/login.php');
        exit();
    }
?>

РЕДАКТИРОВАТЬ

Хорошо, вотмоя попытка случайной соли.Сначала создайте соль для вставки в таблицу:

define('SALT_LENGTH', 15);
function createSalt()
{
    $key = '!@#$%^&*()_+=-{}][;";/?<>.,';
    $salt = substr(hash('sha512',uniqid(rand(), true).$key.microtime()), 0, SALT_LENGTH);
    return $salt;
}
$salt = createSalt()
//More prep for entering into table...

Затем создайте хэш со случайной солью:

$hash = hash('sha256', $salt . $pw); //$pw is the cleaned user submitted password

Когда пользователь входит в систему, сравните сохраненный хеш, используяхранится случайно сгенерированная соль:

$loginHash = hash('sha256', $dbSalt . $pw);
if ($loginHash == $dbHash) {
    //Logged in
} else {
    //Failed
}

Как это выглядит?

Ответы [ 5 ]

8 голосов
/ 03 января 2011

Хорошо, вот несколько:

  1. Вы не должны больше использовать md5. Вы можете , но есть лучшие методы (например, sha512, используемый с функцией hash()).

  2. Я бы также использовал НАМНОГО более длинную статическую соль. Я предлагаю по крайней мере 64 символа (в конце концов, это минимальные накладные расходы для вычисления при записи, но гораздо сложнее угадать).

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

  4. Не доверяйте и не действуйте на основе IP-адреса для чего-либо невременного. Большинство интернет-провайдеров используют форму NAT, при которой более одного пользователя будет видно с одного IP-адреса (и это станет более распространенным с исчерпанием пространства имен IPv4). Если вы хотите ограничить скорость или временно заблокировать IP-адреса, хорошо. Но не запрещайте их ...

  5. Ваша функция clean() должна сначала проверить строку или сделать ее строкой: (* ​​1030 *). Это совсем не мешает внедрению sql. Но вызов stripslashes необходим (именно так, как он у вас есть), чтобы позволить коду работать на серверах с установленным magic_quotes_gpc (который попытается избежать кавычек для вас) ... Так что держите это в стороне.

  6. Отформатируйте ваш код лучше. Создать функции для обработки связанных задач. Таким образом, у вас нет 75 строк процедурного кода для просмотра, чтобы понять, что происходит. Еще лучше, оберните это в классе и переместите общие задачи (доступ к базе данных, и т. Д.) В их собственные классы. И не забывайте делать отступ правильно. Читаемость важна, так что не используйте ярлыки ...

Редактировать: Что касается проверки пароля, вы сначала выбираете соленый хеш, а затем заново вычисляете хеш с сохраненной солью. (Функция makeSaltedHash, которую я покажу ниже, дополнительно использует то, что называется Key Stretching .

function validatePassword($password, $hash) {
    list($oldHash, $salt) = explode(':', $hash, 2);
    $newHash = makeSaltedHash($password, $salt);
    return $hash == $newHash;
}

function makeSaltedHash($password, $salt = '') {
    if (empty($salt)) {
        $salt = makeRandomSalt(mt_rand(64, 128));
    }
    $hash = hash('sha512', $password . $salt . SALT);
    for ($i = 0; $i < 50; $i++) {
        $hash = hash('sha512', $password . $salt . SALT . $hash);
    }
    return $hash . ':' . $salt;
}

function makeRandomSalt($length = 64) {
    $salt = '';
    for ($i = 0; $i < $lenght; $i++) {
        $salt .= chr(mt_rand(33, 126));
    }
    return $salt;
}
4 голосов
/ 03 января 2011

От меня два совета:

  • Не используйте черточки или магические кавычки для предотвращения SQL-инъекций.Используйте параметры PDO.Без исключений.
  • Используйте разные соли для каждого пользователя.Соль не секрет, храните ее в БД вместе с пользовательской записью.Использование одной соли для всех делает вашу базу данных более уязвимой.

Не связано, но часто упускается из виду: не ограничивайте длину пароля для своих пользователей.Я видел слишком много веб-сайтов, которые накладывают произвольное ограничение (например, 12 символов) на длину пароля, но затем имеют смешные правила сложности ( «по крайней мере, один верхний, один нижний, цифра и специальный символ, но не»).<','> '" или такая ерунда).Это очень враждебно, избегайте этого.

1 голос
/ 03 января 2011
  1. Вы используете MD5, который больше не считается "безопасным".

  2. Вам действительно нужна более длинная соль.

Чтобы эффективно использовать соление, см. Этот вопрос .

0 голосов
/ 03 января 2011

Вы не должны банить - просто вставьте recaptcha после 2 неудачных попыток, но если вы действительно хотите использовать memcache для хранения ip. set ($ ip, true, false, $ secondsTTL); позже вы проверяете с помощью get ($ ip) - установите TTL на 2 часа. Вы также можете поместить все в функции и выяснить, что вы хотите использовать для строковых или двойных кавычек. ;)

все вместе выглядит так, как будто это будет работать, но его действительно трудно читать, а некоторые вещи излишни.

0 голосов
/ 03 января 2011

В clean() вы должны добавить mysql_real_escape_string().

В зависимости от вашего сервера и других настроек, $_SERVER['REMOTE_ADDR'] может быть пустым.

Я бы порекомендовал создать простую функцию перенаправления, чтобы предотвратить ошибки при забвении exit после перенаправления header().

Пример:

function redirect($url) {
  if (!headers_sent()) {
     header('Location: '.$url);
  } else {
     // echo $url;
  }
  exit;
}
...