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 Answers 2

Reset to default
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\$
7
  • \$\begingroup\$ Never knew about CSRF can you explain more? \$\endgroup\$ Commented Apr 6, 2016 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\$ Commented Apr 6, 2016 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\$ Commented Apr 6, 2016 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\$ Commented Apr 6, 2016 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\$ Commented Apr 7, 2016 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\$
3
  • \$\begingroup\$ Should I call session_unset() before or after destroy? \$\endgroup\$ Commented Apr 6, 2016 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\$ Commented Apr 6, 2016 at 21:36
  • \$\begingroup\$ Oh ok I wont bother using it then xD \$\endgroup\$ Commented Apr 6, 2016 at 22:07

You must log in to answer this question.