Безопасна и правильна ли такая регистрация?

Question

[Donald Dak]

В php новичок, учусь на локалке и вот задался таким вопросом правильно ли я делаю, хотелось бы узнать какие ошибки и на сколько безопасна такая регистрация

$my = mysqli_connect("localhost", "root", "") or die("Ошибка");
mysqli_select_db($my, "user");

if (isset($_POST)) {
    $name = htmlspecialchars(mysqli_real_escape_string($my,$_POST['name']));
    $login = htmlspecialchars(mysqli_real_escape_string($my,$_POST['login']));
    $password = htmlspecialchars(mysqli_real_escape_string($my,md5($_POST['password'])));
    $captcha = $_POST['captcha'];
    $query = mysqli_query($my,("SELECT * FROM `use` WHERE login='$login'")) or die("Ошибка");
    $user = mysqli_fetch_assoc($query);

if ($name == "" || $login == "" || $password == "") {
        echo "Заполните пустые поля";
}

else if (!preg_match("/^[a-zA-Z0-9\-\_\[\]\(\)]+$/i", $login)) {
        echo "Недопустимые символы";
}

else {
        if ($_SESSION['captcha'] == $captcha && $user['login'] !== $login) {
            $query = mysqli_query($my,"INSERT INTO `use` (`name`, `login`, `password`) VALUES ('$name', '$login', '$password')") or die("Ошибка");
            echo "Вы успешно зарегистрировались";
        }
        else if ($user['login'] == $login) {
            echo "Такой логин уже зарегистрирован";
        }
        else {
            echo "Каптча ведена неверно";
        }
    }
}

Answer 1

[Алексей Николаев]

1. Выкиньте из кода все htmlspecialchars, mysqli_real_escape_string и подобное
2. Используйте PDO и плейсхолдеры (наигоднейшее руководство)
3. Не используйте md5, обновитесь до php 5.5 и используйте Password API (или хотя бы sha, если обновиться нет возможности)
4. Проверяйте получаемые данные на недопустимые символы до любых операций с ними (у вас же сейчас первый запрос проходит до проверки)

После этих 4х пунктов код уже будет гораздо лучше и безопаснее.

Comments

[Donald Dak]

Нужно проверять данные до вот этого?
if (isset($_POST))

[Алексей Николаев]

Don_Donald: эту строчку тоже выкинуть, т.к. $_POST есть всегда. Проверять стоит только ключи в этом массиве, т.е. if(isset($_POST['name'])) и т.д.

А в целом же данные стоит проверять сразу же после их получения, в вашем примере выше - до строки $query = mysqli_query... Т.е. вы получили name, password, проверили, и только после этого выполняете операции с БД

[Uwe_Boll]

Алексей Николаев: на isset() нужно проверять чекбоксы ибо инпуты с типом text, password всегда isset корректнее проверять с помощью empty()

[Алексей Николаев]

Uwe_Boll: да, вы правы

[Anar4you]

Зачем советовать новичку совсем другой sql driver, если он использует mysqli, ему это видимо удобнее

[Donald Dak]

Anar4you: учился на роликах в ютубе, про PDO даже и не знал, ну если PDO более безопаснее чем mysqli то буду изучать его.

[Алексей Николаев]

Anar4you: удобнее != правильно. mysqli вообще плох для использования в подобном роде, слишком многословен. Можно и mysqli, и postresql, но в виде качественной библиотеки, а не чистого API

Answer 2

[Alexander Litvinenko]

php.net/manual/ru/pdo.prepare.php вместо

$name = htmlspecialchars(mysqli_real_escape_string($my,$_POST['name']));
$login = htmlspecialchars(mysqli_real_escape_string($my,$_POST['login']));
$password = htmlspecialchars(mysqli_real_escape_string($my,md5($_POST['password'])));
$captcha = $_POST['captcha'];
$query = mysqli_query($my,("SELECT * FROM `use` WHERE login='$login'")) or die("Ошибка");
$user = mysqli_fetch_assoc($query);

и пароль захешируйте

Comments

[Алексей Уколов]

Пароль хешируется, но без соли и давно устаревшим md5.

[Alexander Litvinenko]

Алексей Уколов: да, неувидел, сильно много этих htmlspecialchars(mysqli_real_escape_string

Answer 3

[romy4]

минимально против sql инъекций выстоит. но безопасность дело очень широкое и далеко не заканчивается на экранировании спецсимволов

Answer 4

[Рустамка Воронцов]

Согласен с Алексей Николаев.
А лучше учится на фрейворке и работать с MVC, если желание есть за неделю разберешься, могу предложить laravel 5

Answer 5

[Donald Dak]

Всем спасибо!

Answer 6

[Anar4you]

Советую для таких несложных регистраций использовать YII или CodeIgniter (последний сейчас обновляется)