1
\$\begingroup\$

Is there anything I need to add or is this the best security possible?

<?php

//Get Sessions Prepared;
session_start();

//If logged in, Logout;
if(!empty($_SESSION['username'])) {

    //Remove Sessions to Logout the user;
    $params = session_get_cookie_params();
    setcookie(session_name(), '', time() - 42000,
        $params["path"], $params["domain"],
        $params["secure"], $params["httponly"]
    );
    session_destroy();

} else {

    //If not logged in redirect to Homepage;
    header("Location: http://gameshare.io/login");

}

?>
\$\endgroup\$
2
\$\begingroup\$

Is there anything I need to add or is that the best security im looking at?

There really isn't much that you can do wrong here. You correctly tell the browser to delete the cookie, and there isn't more that you can do.

But if this is all your logout code, you are vulnerable to CSRF logout. It is debatable if this is a security issue; some argue that it is not, as cookies may be forcefully deleted by overflowing the cookie jar, but that's really a browser issue that may or may not exist on the client side, so to be on the save side, you should add CSRF protection.

As a matter of best practice, you should also always die after a header redirect, as a client doesn't have to follow the redirect and code afterwards will be executed. This doesn't seem to be an issue now, but it may be in the future.

Other than that, your code is fine, but I would remove all your comments, they are just repeating what the code already says. If you want structured and named code blocks, just add functions.

\$\endgroup\$
  • \$\begingroup\$ Never knew about CSRF can you explain more? \$\endgroup\$ – user100593 Apr 6 '16 at 21:32
  • \$\begingroup\$ In that case, you should definitely read up on it. CSRF logout is a very minor issue for most applications, but CSRF across an application is a serious security issue. With a CSRF attack, an attacker can perform any action for an already authenticated user if that user visits a website that is partially attacker controlled. For more information, see the OWASP page on CSRF \$\endgroup\$ – tim Apr 6 '16 at 21:39
  • \$\begingroup\$ Read up on it but I honestly dont know how that tiny script is CSRF vulnerable. Could you give me an example on preventing it? \$\endgroup\$ – user100593 Apr 6 '16 at 22:06
  • \$\begingroup\$ @ShinyMK Any script that accepts requests that change state is always vulnerable to CSRF if there is no CSRF protection in place. For protection, see the OWASP page on CSRF protection. The recommended approach is the synchronizer token pattern (when starting a session, you store a random token in it, and on every request that changes state, you send that token as well - via GET or POST - and compare it to the stored token). \$\endgroup\$ – tim Apr 6 '16 at 22:13
  • \$\begingroup\$ Well I still dont really understand could you like show me exactly how I can exploit my own page? Maybe then I could understand \$\endgroup\$ – user100593 Apr 7 '16 at 6:37
1
\$\begingroup\$

Yes, that will do the job. A couple of notes:

  1. Firstly, session values in $_SESSION will still be set after calling session_destroy() (for the remainder of the script's execution). If you'd like to clear the $_SESSION variable too, call session_unset() as well.

  2. You don't actually have to unset the session cookie if you don't want to (as once you've called session_destroy(), the user's session cookie becomes completely meaningless and therefore cannot be used for anything). But if you do, the method you have used will work as expected.

\$\endgroup\$
  • \$\begingroup\$ Should I call session_unset() before or after destroy? \$\endgroup\$ – user100593 Apr 6 '16 at 21:32
  • \$\begingroup\$ It doesn't matter which way round you do it. (BTW, If your script doesn't do anything after logging the user out, a call to session_unset isn't necessary.) \$\endgroup\$ – Emily Shepherd Apr 6 '16 at 21:36
  • \$\begingroup\$ Oh ok I wont bother using it then xD \$\endgroup\$ – user100593 Apr 6 '16 at 22:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy