0

I am trying to add "Stay Connected" feature on login and my idea was to create a checkbox which has a value of "yes" and if the checkbox is set and user password and email is found in database then I assigned $_COOKIE['usigh-ses'] to $row['id'].

In my application I am using the user id(AUTO INCREMENT) to identify each user and corresponding details.

I then later login and check my browser and I saw the cookie name and content which happens to be user id so am confused.

Please suggest to me whether or not this is a good practice. If not suggest which proper way i could achieve this.

Can someone use the COOKIE and value to login or hijack my application without passing through the login process?

below is my whole login script:

<?php

    //require connection file
    require('include/dbc.php');
    include ('include/functions.php');
    loggedin_type();


    //redirect iff session  or cookie is already set and its not empty
    if(loggedin()){
    header("location:home?ref=log");
    exit();

    }




    // create empty variables to hold data
    $email = $password =$errors= $name= $name2= $u_avatar="";
    $emailErr = $passwordErr ="";

    $passwordbox =false;
    $emailbox =true;

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


    if(empty($_POST['email']) || ctype_space($_POST['email'])){
    $emailErr ="Please enter your email address.";
    }else{
    $email = trim(strtolower($_POST['email']));

    //Validate for correct email
    if(!filter_var($email,FILTER_VALIDATE_EMAIL)){
    $emailErr ="Enter a valid email address.";
    }
    } //end of email 




    if(ctype_space($_POST['password'])){
    $passwordErr ="Please enter a valid password.";
    //errors ='<div class="topalerts"> Go ahead and enter your password</div>';


    }



    //Recheck validation 
    if($email !="" && !ctype_space($email) && filter_var($email,FILTER_VALIDATE_EMAIL)){


    //AsK database questions
    $sql = "SELECT * FROM $table_name WHERE Email ='$email' LIMIT 1";
    $result = mysqli_query($dbc_conn,$sql);
    $numrows =mysqli_num_rows($result);



    if($numrows > 0){
    while( $row =mysqli_fetch_assoc($result)){
    $db_email = $row['Email'];


    if($email == $db_email){

    if($row['avatar'] !=NULL){
    $image = $row['avatar'];
    $image_url = "uploaded/$image";
    if(file_exists($image_url)){
    $u_avatar = $row['avatar']; 
    }else{

    //Default profile avatar because OF ERROR OR FILE DO NOT EXIST  
    $u_avatar = "blank-profile.png";
    }

    }else{
    //Default profile avatar because row AVATAR is NULL 
    $u_avatar = "blank-profile.png";

    }






    //hide email div, show password div 
    $name = $row["FirstName"][0];
    $name2 = $row['FirstName'];
    $passwordbox =true;
    $emailbox =false;

    //check for valid password
    if(!empty($_POST['password']) and !ctype_space($_POST['password'])){

    $password = md5($_POST['password']);
    if( $password == $row['Password']){

    $logginok =TRUE;
    if($logginok ==TRUE){

    //remember me feature
    if(isset($_POST['remember'] ) && $_POST['remember']=="yes"){
    setcookie("usigh-ses",$row['id'],time()+ 172800);

    $rand = openssl_random_pseudo_bytes(16);
    $serial = bin2hex($rand);           
    //this user is online
    mysqli_query($dbc_conn,"UPDATE $table_name SET active=1 WHERE id ={$row['id']} ");
    header("location:home?search=$serial");

        }else{

    $rand = openssl_random_pseudo_bytes(16);
    $serial = bin2hex($rand);           
    //this user is online
    mysqli_query($dbc_conn,"UPDATE $table_name SET active=1 WHERE id ={$row['id']} ");
    //normal login
    $_SESSION['usigh-ses']=$row['id'];  
    header("location:home?search=$serial");
    exit();

    }






    }






    }else{
    $logginok =FALSE;
    $errors ='<div class="topalerts"> The password you have entered is invalid. 
    Please provide a valid password of your account.</div>';
    $passwordErr = 'The email and password you entered don\'t match. ';

                    }
                }


            }


        }


    }else{


        $errors ='<div class="topalerts"> It seems you are not a registered member
         or your email is incorrect.Try again.</div>';
        $emailErr = "Sorry, your email could not be verified.";

        }



        }//end of recheck
        else{
            $errors ='<div class="topalerts">There were one or more errors in your submission.
             Please correct the mark fields below.</div>';
            }


    } //end of main submit
     ?>

Thanks for your good responses.

James Favour
  • 97
  • 1
  • 2
  • 9
  • Don't store anything that the user should not be able to alter in a cookie. For authentication, just store the session ID, and store the session data server-side. – Alexander O'Mara Apr 12 '16 at 07:04
  • @AlexanderO'Mara , Please looking at my code can you write a code that can best suit what you are saying ?? ....thank you. – James Favour Apr 12 '16 at 07:10
  • @AlexanderO'Mara "Just use the session" (which is what you've effectively said) is not a remember-me implementation (the scope of what maly is trying to do). – AD7six Apr 12 '16 at 08:02
  • @AD7six It can be, so long as the session I'd is stored in a non-session cookie, and the session id does not expire for the duration of the "remember me" time.. The main pont was, don't store something like `loggedinuser=123` client-side. – Alexander O'Mara Apr 12 '16 at 15:49

1 Answers1

1

Storing user ID in the cookie and authenticating user just on that info is a bad idea, because you generate the user ID sequentially (AUTOINCREMENT) so it is very easy to guess, allowing the attacker to impersonate another user.

The correct way is to generate a random identifier, associate it with the user and only store the random ID in the cookie. This can be done easily through $_SESSION (session actually is implemented by creating a random identifier and storing it, often in a cookie), or you can do so manually by storing the random ID in database.

Note that the randomness of the generated ID is crucial part of the security here and there have been attacks on session in the past that were based on PHP generating too predictable pseudo-random numbers.

Jiri Tousek
  • 12,211
  • 5
  • 29
  • 43
  • `Note that the randomness of the generated ID is crucial part` careful, that sounds a lot like "rely on security through obscurity". There are more detailed descriptions around, e.g. http://stackoverflow.com/a/244907/761202 – AD7six Apr 12 '16 at 08:01
  • @AD7six This is not security through obscurity (i.e. relying on keeping the implementation secret), it's security through generating a secret "key" in a way that's non-guessable (random) and keeping it secret. Good link nevertheless. – Jiri Tousek Apr 12 '16 at 08:07