From e04d4580baaeeb98728462095c7c30f513624bf3 Mon Sep 17 00:00:00 2001 From: PaulT Date: Tue, 27 Mar 2018 21:39:18 -0400 Subject: [PATCH] Meet coding standards and more commentary. Change: $name => $Name $cookie_info => $CookieInfo ... to meet coding standards. Also, add more commentary/information details. --- Logout.php | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/Logout.php b/Logout.php index 5fc84ddea..174fbbacd 100644 --- a/Logout.php +++ b/Logout.php @@ -7,42 +7,47 @@ session_destroy(); // Do not assume php.ini defaults are in use on other systems using this app. -$name = session_name(); +$Name = session_name(); -// We do not use session_set_cookie_params(), fetch the php.ini values. +// We do not use session_set_cookie_params(), so 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(); +$CookieInfo = 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]); +setcookie($Name, '', 1, $CookieInfo['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 + 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. + The cookie's name (which comes from session.name, or session_name() could be + used to override) will appear to remain with a newly regenerated value, + which it does, but this is due to OUR handling as we have tight-coupling to + certain $_SESSION data in files index.php and the included header.php. + 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. + in index.php (followed by exit, 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