CODEWITHSUNDEEP
phpsecuritysessionsession-variables
Jonan
Jonan

Reputation: 2536

Session validation destroys session

This is my Session class:

class Session {

    const SESSION_VALIDATOR = 'validSession';

    /**
     * Starts the session
     */
    public static function init() {
        session_start();

        if (self::get(self::SESSION_VALIDATOR) !== true) {
            session_unset();
            session_destroy();
            session_start();
            self::set(self::SESSION_VALIDATOR, true);
        }

        session_regenerate_id(false);
    }

    /**
     * Sets a value in the session
     * 
     * @param string|int $key
     * @param mixed $value
     */
    public static function set($key, $value) {
        $_SESSION[$key] = $value;
    }

    /**
     * Gets a value from the session
     * 
     * @param string|int $key
     * @return mixed
     */
    public static function get($key) {
        if (isset($_SESSION[$key]))
            return $_SESSION[$key];
        else
            return false;
    }

    /**
     * Destroys the session
     */
    public static function destroy() {
        unset($_SESSION);
        session_destroy();
    }

}

At random times, the SESSION_VALIDATOR variable in the session seems not to be set, even though it isn't changed, and the session randomly destroys - even while a user is logged in. What causes this?

Update 1:
This appears to occur only at my localhost environment (WAMPServer 2.5), and not on my shared hosting account

Update 2:
It doesn't seem to be occur because the Session::destroy() method is called somewhere at accident, as when I throw an exception inside the method, the error occurs without the exception being thrown

Update 3:
Ignore update #1 - it also happens on my shared hosting account

Update 4
I tried andpei's answer, and the problem still occurs. This is the new Session class:

class Session {

    const SESSION_STARTED = true;
    const SESSION_NOT_STARTED = false;
    const SESSION_VALIDATOR = 'validSession';

    private $sessionState = self::SESSION_NOT_STARTED;
    private static $instance;

    private function __construct() {

    }

    public static function getInstance() {
        if (!isset(self::$instance)) {
            self::$instance = new self;
        }

        self::$instance->startSession();

        return self::$instance;
    }

    public function startSession() {
        if ($this->sessionState == self::SESSION_NOT_STARTED) {
            $this->sessionState = session_start();
        }

        if (self::get(self::SESSION_VALIDATOR) !== true) {
            $this->destroy();
            session_start();
            self::set(self::SESSION_VALIDATOR, true);
        }

        session_regenerate_id(false);

        return $this->sessionState;
    }

    public function set($name, $value) {
        $_SESSION[$name] = $value;
    }

    public function get($name) {
        if (isset($_SESSION[$name])) {
            return $_SESSION[$name];
        }
    }

    public function __isset($name) {
        return isset($_SESSION[$name]);
    }

    public function __unset($name) {
        unset($_SESSION[$name]);
    }

    public function destroy() {
        if ($this->sessionState == self::SESSION_STARTED) {
            $this->sessionState = !session_destroy();
            unset($_SESSION);

            return !$this->sessionState;
        }

        return false;
    }
}

This made me think the error was in the SESSION_VALIDATOR part:

if (self::get(self::SESSION_VALIDATOR) !== true) {
    $this->destroy();
    session_start();
    self::set(self::SESSION_VALIDATOR, true);
}

, so I removed it, and now the error doesn't occur anymore. Is this session validation really necessary? Why or why not? And if it's smart to keep it, how can the error be resolved?

Upvotes: 3

Views: 493

Answers (1)

admirableadmin
admirableadmin

Reputation: 2759

It's a design problem, because you unintentionally create more than one instance of the Session object, which then all may have a different value for SESSION_VALIDATOR.

To avoid this problem you should use in OOP the Singleton Pattern. Instead of creating an instance by = new Session the getInstance() method will return always the same instance object.

You could try this example to create a class Session with a Singleton.

Update to comment

If you do not instantiate the Session object, then it may get deleted by the Garbage Collector, because there is no reference to it. As a result SESSION_VALIDATOR is also deleted. In that case you destroy the session (_unset,_destroy,_start), which logs off an user.

You could try the example from above, which will solve the problem by use the singleton pattern.

Than the whole object is stored as a static value.

Update to question about SESSION_VALIDATOR

It's a design problem, because you store a state at server-side which depends on a session-state at client-side, i.e. the garbage collector runs (server) or the browser deletes cookies or logs off (client).

In theory this means that HTTP is stateless and a session is used to overcome this problem. In practice this means you have to read a value from the session and decide if a login is valid or not.

So, your validation has to be a function instead of a SESSION_VALIDATOR variable, which returns a boolean depending on the session state, its values and your user database, i.e with a Singleton.

But do not store something like login=true in your session. Use values which you could validate with your user database.

Also take a look at OWASP PHP Security Cheat Sheet

similar questions:

Upvotes: 2

Related Questions