Есть ли какие-либо уязвимости безопасности в этом коде регистрации PHP?

Question

Можете ли вы эксперты дать мне несколько соображений по этому коду? Какая-то дыра в безопасности, которую я пропустил? Вы видите какие-либо потенциальные угрозы? Что я могу сделать лучше?

Я все еще учусь :) Спасибо

<?php

if (isset($_POST['username'])) {

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string($_POST['password']);
$password2 = mysql_real_escape_string($_POST['password2']);
$encrypted_password = md5($password);


// remove eventuakl space
foreach($_POST as $key => $val) $_POST[$key] = trim($val);


// check if username is taken
$query = mysql_query("SELECT COUNT(*) FROM users WHERE username = '$username'");
if (mysql_result($query, 0) > 0) {
$reg_error[] = 0;
}

// make sure username only cosist of at least 3 letters, numbers or _ -
if (!preg_match('/^[a-zA-Z0-9_-]{3,}$/', $username)) {
$reg_error[] = 4;  
}


// check for empty fields
if (empty($username) || empty($password) || empty($password2)) {
$reg_error[] = 2;
}

// check if the passwords match
if ($password != $password2) {
$reg_error[] = 3;
}

// save if error is unset
if (!isset($reg_error)) {
mysql_query("INSERT INTO users (username, password, registered, registration_ip)
             VALUES('$username', '$encrypted_password', '".time()."', '".$_SERVER['SERVER_ADDR']."')");

$_SESSION['id'] = mysql_insert_id();
header('refresh: 3; url=/home');

}


}
?>

login.php

if (isset($_POST['username'])) {

$username = mysql_real_escape_string($_POST['username']);
$password = mysql_real_escape_string($_POST['password']);
$md5_password = md5($password); 

$query = mysql_query("SELECT id FROM users WHERE username = '$username' and password = '$md5_password'");


if (mysql_num_rows($query) == 0) {
header("Location: ".$_SERVER['REQUEST_URI']."");
exit;
}

// set session
$_SESSION['id'] = mysql_result($query, 0, 'id');
header("Location: /");
exit;

Answer 1

Вы не соль пароль.

Кроме того, md5() считается недостаточно сильным для хеширования паролей.

Вместо этого используйте hash('sha256', $password).

Answer 2

Вам нужно посолить пароль.

Это не в том месте. Переместите его вверх на несколько строк, прежде чем использовать переменные $ _POST.

// remove eventuakl space
foreach($_POST as $key => $val) $_POST[$key] = trim($val);

Вы экранируете поля пароля без причины. Они не отправляются в базу данных. md5 ($ password) собирается в базу данных и не экранируется.

РЕДАКТИРОВАТЬ: На стороне входа, вы должны обрезать все, что вы обрезать на стороне регистрации.

Answer 3

Маленькие таблицы Бобби доставят вам много головной боли, поскольку вы не проверяете действительность имени пользователя.

Answer 4

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

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

Answer 5

Ваша проверка ошибок не в порядке. Я бы сделал проверку ошибок в следующем порядке:

1. Проверить наличие пустых полей
2. Убедитесь, что поля имеют допустимые значения (фильтрация ввода)
3. Перед использованием полей в SQL экранируйте поля с помощью mysql_real_escape_string *
4. Проверка пользователя в таблице SQL

Если вы обнаружили ошибку, не продолжайте дальнейшие проверки. Защитите каждую проверку ошибок, аналогичную вашей проверке в заключительном утверждении INSERT.

У вас нет правок в полях пароля, кроме использования mysql_real_escape_string?

Вы должны выполнить mysql_connect перед использованием mysql_real_escape_string. mysql_real_escape_string будет использовать соединение для определения набора символов соединения. Набор символов будет определять, какие символы нужно убежать.

Answer 6

Вы должны использовать параметры, а не строить свой sql динамически. Это поможет предотвратить атаки SQL инъекций. Маленькие заколочные столики доставят тебя.