Secure login form, or at least I hope so

Question

I've written a new email form. I've implemented stuff suggested by some good folk around here and so I tried my best to make it both simple and secure, though I'm not sure if I succeeded in the latter considered that I'm stuck with older MySQL API for now. I feel like I have to explain why. It's because the whole site is using mysql and I have no time to switch to PDO, at least not for now.

I didn't run password through !preg_matchthough, not sure if that makes the input vulnerable to some kind of attack? I got a sense that when using !password_verify I can just sit back and relax.

Login form uses nickname, email and password to sign in.

Here's my code:

if (@$_POST['login']) {
    $nickname = mysql_real_escape_string($_POST['nickname']);
    $email = $_POST['email'];
    $password_input = $_POST['password'];
// validation 1 ------- //
    else if (filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
    $error = "Wrong nickname password or email.";
    }
    else if (!preg_match("/^[a-zA-Z0-9]{3,30}$/",$_POST['nickname'])) {
    $error = "Wrong nickname password or email.";
    }
// validation 2 ------- //  
        else { //password check
        $password_query = mysql_query ("SELECT password FROM userbase WHERE email='$email' && nickname='$nickname'"); 
        $password_actual = mysql_result ($password_query, 0);
        if (!password_verify($password_input, $password_actual)) {
        $error = "Wrong nickname, password or email.";
        }
            else {
            LOGIN SUCCESSFUL

Is !preg_match for $_POST['nickname'] needed if it's escaped by mysql_real_escape_string? (security vise)

Your code is extremely vulnerable to SQL injection, even with `mysql_real_escape_string`. It's recommended that you use PDO. — The Codesee, Apr 22 '16 at 14:21
@Codesee read several of the OP's posts. He is not in a position to change MySQL API's at the moment. — Jay Blanchard, Apr 22 '16 at 15:49
@SaulTigh I have to ask: what is it you're trying to protect? You have to consider the information you're trying to protect vs. the level of protection that you want to add. You may be doing too much and gaining too little for your efforts. — Jay Blanchard, Apr 22 '16 at 15:51
The website stores user's comments, private messages, passwords, nicknames, email addresses and it has a follow feature (users can follow each other). Nothing more. Maybe I'm too paranoid considered the casual info the website stores, but I just like security. I mean, even if someone removes my DB through SQL injection, it's not a big deal since I backup the data every 12hours. I don't know, what do you think? What would you do? — Saul Tigh, Apr 22 '16 at 15:58

score 1 · Answer 1 · edited May 23 '17 at 12:07

1

MYSQL and MYSQLI functions are extremely vulnerable to SQL injection, even with mysql_real_escape_string.

How can I prevent SQL-injection in PHP?

I have converted your code from MYSQLI to PDO, which are SQL statements that are sent to and parsed by the database server separately from any parameters.

$con = new PDO('mysql:host=localhost;dbname=name', 'user', 'pass');

if ($_POST['login']) {
$nickname = $_POST['nickname'];
$email = $_POST['email'];
$password_input = $_POST['password'];
else if (filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
$error = "Wrong nickname password or email.";
}
else if (!preg_match("/^[a-zA-Z0-9]{3,30}$/",$_POST['nickname'])) {
$error = "Wrong nickname password or email.";
}else{ 
$password_query = $stmt = $con->prepare("SELECT password FROM userbase WHERE email=:email && nickname=:nickname"); 
$stmt->bindParam(':email', $email);
$stmt->bindParam(':nickname', $nickname);
$stmt->execute();
if($password_query->rowCount() > 0) {
$row = $stmt->fetch();
if(!password_verify($password_input, $row['password'])) {
$error = "Wrong nickname, password or email.";
}else{
 //LOGIN SUCCESSFUL
}

edited May 23 '17 at 12:07

Community

1
1

answered Apr 22 '16 at 14:32

The Codesee

3,714
5
38
78

can I use this even if the rest of the website is using old mysql? – Saul Tigh Apr 22 '16 at 15:34
1

Most likely not - it's not recommended to mix them. I recommend you take a while to read about PDO and update your code. http://php.net/manual/en/book.pdo.php – The Codesee Apr 22 '16 at 15:44
Ok thanks, though unfortunately I can't switch right now (definitely gonna do it by the end of the year, just not at the moment). I'm kinda confused how the hell do all the websites that are still using old mysql stopping SQL injection attacks if mysql is so defenseless against SQL injections? There must be at least some way to limit the sql injections. I don't need bullet proof defense, but something that will stop average / basic attacks, if you get what I'm saying. – Saul Tigh Apr 22 '16 at 15:52
Hi Codesee, in reply to that deleted question comment. There is no security gain - you are using PDO style for binding named parameters, the other was MySQLi, read http://code.tutsplus.com/tutorials/pdo-vs-mysqli-which-should-you-use--net-24059 for more info – Matt Apr 22 '16 at 16:32
@Matt Ah, I understand. Thanks for that. – The Codesee Apr 22 '16 at 16:58

Secure login form, or at least I hope so

1 Answers1