From b163d75c1f7ef9cfe863fb555c0c744ac8a0f821 Mon Sep 17 00:00:00 2001
From: PaulT <TurboPT@users.noreply.github.com>
Date: Fri, 9 Mar 2018 00:21:10 -0500
Subject: [PATCH] Logout cookie handling

Change the cookie lifetime to one, and retrieve other parameters needed for cookie removal.
---
 Logout.php | 43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

diff --git a/Logout.php b/Logout.php
index 02db69f8f..5fc84ddea 100644
--- a/Logout.php
+++ b/Logout.php
@@ -1,13 +1,48 @@
 <?php
 
-
-$AllowAnyone=True; /* Allow all users to log off  */
-
 include('includes/session.php');
 
 // Cleanup
 session_unset();
 session_destroy();
-setcookie('PHPSESSID',"",time()-3600,'/');
 
+// Do not assume php.ini defaults are in use on other systems using this app.
+$name = session_name();
+
+// We do not use session_set_cookie_params(), fetch the php.ini values.
+// This information is needed for proper handling to avoid the "common pitfalls"
+// referenced within the PHP setcookie() documentation:
+// "Cookies must be deleted with the same parameters as they were set with."
+$cookie_info = session_get_cookie_params();
+
+/////////////////// OWASP
+
+// Destroy the cookie handling based on OWASP recommendations:
+// https://www.owasp.org/index.php/PHP_Security_Cheat_Sheet#Proper_Deletion
+
+setcookie($name, '', 1, $cookie_info['path']);
+setcookie($name, false);
+unset($_COOKIE[$name]);
+
+/////////////////// END OWASP
+
+/*
+    Testing the cookie destroy handling with the three calls above appears to be
+    OK. However, one could easily be misled to assume that the cookie is not
+    destroyed while viewing the cookies in the browser's debugger while using
+    our app after logout.
+
+    The cookie's name (which comes from session.name) will appear to remain with
+    a newly regenerated value, which it does, but this is due to OUR handling.
+    Post-logout handling is redirected to index.php, and the index.php file
+    includes session.php. Function session_start() is called from session.php
+    and that call recreates the cookie with same name but with a new cookie
+    value. (basically, the cookie is removed and then recreated)
+
+    So, to test that the cookie is actually removed, I added a header redirect
+    in index.php (before including session.php) to another local file that does
+    NOT have a session_start() call. When watching the cookies in the browser's
+    debugger, one can witness that the cookie with the session name is removed.
+    I verifed this behavior with Firefox and Chrome.
+*/
 ?>
\ No newline at end of file