CODEWITHSUNDEEP
phpsessioncookiessession-cookies
Tural Ali
Tural Ali

Reputation: 23290

Working with sessions and cookies in PHP

During sign-in I'm using following function to set cookies and session

protected function validateUser($userid, $ckey=0, $rememmber=0) {
    session_start();
    session_regenerate_id(true); //this is a security measure
    $_SESSION['user_id'] = $userid;
    $_SESSION['HTTP_USER_AGENT'] = md5($_SERVER['HTTP_USER_AGENT']);
    if (isset($remember) && $rememmber == 'on') {
        setcookie("user_id", $_SESSION['user_id'], time() + 60 * 60 * 24 * COOKIE_TIME_OUT, "/");
        setcookie("user_key", sha1($ckey), time() + 60 * 60 * 24 * COOKIE_TIME_OUT, "/");
    }
    return true;
}

Then on secure user pages, checking for user_id using user_id to fetch all important data about user from db

public function protect() {
        session_start();

        /* Secure against Session Hijacking by checking user agent */
        if (isset($_SESSION['HTTP_USER_AGENT'])) {
            if ($_SESSION['HTTP_USER_AGENT'] != md5($_SERVER['HTTP_USER_AGENT'])) {
                $this->signout();
                exit;
            }
        }

// before we allow sessions, we need to check authentication key - ckey and ctime stored in database

        /* If session not set, check for cookies set by Remember me */
        if (!isset($_SESSION['user_id'])) {
            if (isset($_COOKIE['user_id']) && isset($_COOKIE['user_key'])) {
                /* we double check cookie expiry time against stored in database */

                $cookie_user_id = $_COOKIE['user_id'];
                               $stmt = $this->db->prepare("select `ckey`,`ctime` from `users` where `id` =?") or die($this->db->error);
            $stmt->bind_param("i", $cookie_user_id) or die(htmlspecialchars($stmt->error));
            $stmt->execute() or die(htmlspecialchars($stmt->error));
            $stmt->bind_result($ckey, $ctime) or die($stmt->error);
            $stmt->close() or die(htmlspecialchars($stmt->error));
                // coookie expiry
                if ((time() - $ctime) > 60 * 60 * 24 * COOKIE_TIME_OUT) {
                    $this->signout();
                }
                /* Security check with untrusted cookies - dont trust value stored in cookie.       
                  /* We also do authentication check of the `ckey` stored in cookie matches that stored in database during login */

                if (!empty($ckey) && is_numeric($_COOKIE['user_id']) && $_COOKIE['key'] == sha1($ckey)) {
                    session_regenerate_id(); //against session fixation attacks.

                    $_SESSION['user_id'] = $_COOKIE['user_id'];
                    $_SESSION['HTTP_USER_AGENT'] = md5($_SERVER['HTTP_USER_AGENT']);
                } else {
                    $this->signout();
                }
            } else {
                if ($page != 'main') {
                    header('Location:' . wsurl);
                    exit();
                }
            }
        }
    }

I wonder, is that enough to store only user_id and user_agent in cokkies and session for security purposes? If not, what else?

Upvotes: 1

Views: 1000

Answers (2)

Dmitriy Shasherin
Dmitriy Shasherin

Reputation: 166

$rs_ctime = $this->db->query("select `ckey`,`ctime` from `users` where `id` = -> '$cookie_user_id'<-")

Seems like here is SQL-injection. If you pass variable from GET, POST, COOKIE directly to sql-query it is a sign of injection.

Usually it is enough to store user_id and user_agent. But some payment services sometimes use verification by phone sending sms with some code and checking it. But it is usually for very safe services

Upvotes: 4

Authman Apatira
Authman Apatira

Reputation: 4054

Sessions are stored on the server and you're already doing your part regenerating the session key immediately after login, per Wikipedia's Session Hacking Prevention, so I'd say you're good just storing user agent and user id. I would not, however, regenerate the session id after every secure page view (eg, remove session_regenerate_id() from the validateUser() function). Wikipedia recommends only doing it once upon login... anything more than that is kind of just out of control. I would also move session_start() out of the functions though and up top in a nice high place in your code.

Good luck coding!

Upvotes: 3

Related Questions