-2
if($_SERVER['REQUEST_METHOD'] == 'POST') {
    $login = $_POST['login'];
    $password = $_POST['password'];

    //if(isset($login) && isset($password)) {
    if((isset($login) && isset($password)) && preg_match('/^[a-z0-9\-\_]+$/i', $login)) {
        if($result = $db->query("SELECT * FROM `users` WHERE `username`='{$login}'")) {
            if($count = $result->num_rows) {
                while($row = $result->fetch_object()) {
                    $user_name = $row->username;
                    $user_password = $row->password;

                    if(password_verify($password, $user_password)) {
                        setcookie('u_log', $user_name);
                        $_COOKIE['u_log'] = $user_name;
                        setcookie('u_pass', $user_password);
                        $_COOKIE['u_pass'] = $user_password;

                        header("Location: /user/$user_name");
                    } else { $error = 1; $err_message = 'Неверный логин или пароль.'; }
                }
            } else { $error = 1; $err_message = 'Неверный логин или пароль.'; }
        } else { $error = 1; $err_message = 'Неверный логин или пароль.'; }
    }
}

I'm aware I need to escape single and double quotes in user input but don't know if this fully safe solution as there some commands that can be executed anyway. Can somebody analyze my code and describe what I need to do here?

Slightly changed code in the almost top. Now it should be more secure?

O. Jones
  • 103,626
  • 17
  • 118
  • 172
DIES
  • 345
  • 5
  • 14
  • 4
    You don't need to escape anything if you use proper prepared/parameterized queries which will actually protect you against sql injection. Why do you set a cookie with the users password? I'd strongly urge you to just use PHPs built in session system to persist user data instead of managing cookies yourself, and never store the password anywhere – JimL Oct 22 '17 at 13:07
  • 2
    Voting to close, because this belongs on [codereview](https://codereview.stackexchange.com/) – rickdenhaan Oct 22 '17 at 13:09
  • @JimL I din't understood if I have to do anything here. Cookies because of mine ignorance about how to use sessions before, and now this thing working in root of my site, changing everything will be a pain. – DIES Oct 22 '17 at 13:11
  • @DIES you need to start using prepared/parameterized queries instead of concatenating values directly into the query string. I suggest you change to use the session system. Development is painful sometimes. – JimL Oct 22 '17 at 13:12

1 Answers1

3

Secure?

  • As to password hashing and verification, almost. Look up password_needs_rehash() and use it.
  • As to SQL injection, no. Use parameterized queries.
  • As to cookies, no. Your sesssion cookies should have httpOnly and secure flags set. Look up the last two parameters of setcookie() in the php manual.
  • As to putting the plain text of your user's password in a cookie, no, no, a thousand times no! Use a hard-to-guess session id in a cookie, not a high-value secret. Session ids are temporary compared to passwords.
O. Jones
  • 103,626
  • 17
  • 118
  • 172