How to fix this login function

Question

I have these two functions in my PHP code:

function admin_checker($username,$password)
{
    $username=remove_extra_in_string($username);
    $password=remove_extra_in_string($password);
    $q='select * from `admin` where `username`="'.$username.'" and `password`="'.$password.'"';
    $result=@mysqli_query($conn, $q);
    $_COOKIE['username'] = $result["username"];
    $_COOKIE['password'] = $result["password"];
    if(@mysqli_num_rows($result)==1)
        return 1;
    else
        return 0;
}

function remove_extra_in_string($string)
{
    $extra=array('\'','"','`','/','*',';',' ','--');
    $string=str_replace($extra,'',$string);
    return $string;
}

Then checking by this:

if(admin_checker($_COOKIE['username'],$_COOKIE['password'])==1)
{ echo "ok"; } else { echo "not ok"; }

But somehow it not pass, I have no idea what I am doing wrong here?

1: you're open to SQL injection 2: DO NOT STORE PASSWORDS IN COOKIES 3: you're suppressing errors for your mysqli_* function - you'll want these to display. 4: you should be using `password_verify` and `password_hash` for this kinda biz — treyBake, Apr 26 '19 at 11:15
**Never store plain text passwords!** Please use ***PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html)*** to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). ***It is not necessary to [escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. — Jay Blanchard, Apr 26 '19 at 11:29
Stop suppressing errors and for goodness sake, *do not deploy this code into a production environment.* — Jay Blanchard, Apr 26 '19 at 11:30
The code has all the marks of advise taken from way outdated tutorials and quite terrible blog posts. (Please see through https://phptherightway.com/ [partially at least].) — mario, Apr 26 '19 at 11:38

score 0 · Answer 1 · answered Apr 26 '19 at 11:21

0

First and the most important thing: NEVER store passwords as a plain text and NEVER keep user data in COOKIE. Next thing - do not use your own function remove_extra_in_string(), use built-in mysqli_real_escape_string or even better - use prepared statements: https://www.php.net/manual/en/mysqli.quickstart.prepared-statements.php

The case is if you are storing passwords in plain text, you may have more records with the same password and username and it will return value != 1 so it will be FALSE.

It may also fail because you do not have set COOKIES while calling admin_checker() so $username and $password are equal null.

answered Apr 26 '19 at 11:21

Grzegorz Lasak

96
4

Thanks for the info! I will try with mysqli_real_escape_string instead of my remove_extra_in_string, passwords are secured by line $password=md5($password); and cookies are set by line session_start(); It that ok now? – Guy Degre Apr 26 '19 at 11:36
Just use prepared statement instead of any kind of escaping like that. And ***do not use `md5()`***. Use `password_hash()` with `password_verify()` – Qirel Apr 26 '19 at 11:37
@Qirel Ok... great, thanks! – Guy Degre Apr 26 '19 at 11:42
@GuyDegre well, not really, sorry. Use password_hash() - https://php.net/manual/de/function.password-hash.php and password_verify() - https://www.php.net/manual/en/function.password-verify.php. I am personally using BCRYPT for hashing passwords. Also, when User inserts his credentials, get them via POST request and and insert into admin_checker as parameters: admin_checker($_POST['username'], $_POST['password']), and then use prepared statements for your queries. – Grzegorz Lasak Apr 26 '19 at 11:45
@GuyDegre Last part of my previous comment: To set cookies properly, you need to set them by specifying all flagues properly + I recommend to sign them by creating specific hash for them (which you compare every time) to be sure they are set by you and only for your website. https://www.php.net/manual/en/function.setcookie.php – Grzegorz Lasak Apr 26 '19 at 11:45

How to fix this login function

1 Answers1