From fbad6b9b4d175aa82cd35972aa2dcd9ef3ec4346 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C4=8Ciha=C5=99?= Date: Mon, 19 Sep 2016 11:43:08 +0200 Subject: [PATCH] Verify value of access_time to avoid unwanted session extension MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We need to ansure the access_time parameter is in valid range to avoid possibility of remotely extending session validity. Signed-off-by: Michal Čihař --- libraries/plugins/AuthenticationPlugin.php | 22 +++++++++++++++++++ .../plugins/auth/AuthenticationConfig.php | 8 +------ .../plugins/auth/AuthenticationCookie.php | 6 +---- libraries/plugins/auth/AuthenticationHttp.php | 4 +--- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/libraries/plugins/AuthenticationPlugin.php b/libraries/plugins/AuthenticationPlugin.php index 3a435d22f0..6063aa98e0 100644 --- a/libraries/plugins/AuthenticationPlugin.php +++ b/libraries/plugins/AuthenticationPlugin.php @@ -134,4 +134,26 @@ abstract class AuthenticationPlugin public function handlePasswordChange($password) { } + + /** + * Store session access time in session. + * + * Tries to workaround PHP 5 session garbage collection which + * looks at the session file's last modified time + * + * @return void + */ + public function setSessionAccessTime() + { + if (isset($_REQUEST['access_time'])) { + // Ensure access_time is in range <0, LoginCookieValidity + 1> + // to avoid excessive extension of validity. + // + // Negative values can cause session expiry extension + // Too big values can cause overflow and lead to same + $_SESSION['last_access_time'] = time() - min(max(0, intval($_REQUEST['access_time'])), $GLOBALS['cfg']['LoginCookieValidity'] + 1); + } else { + $_SESSION['last_access_time'] = time(); + } + } } diff --git a/libraries/plugins/auth/AuthenticationConfig.php b/libraries/plugins/auth/AuthenticationConfig.php index a98263b94f..77bf980d97 100644 --- a/libraries/plugins/auth/AuthenticationConfig.php +++ b/libraries/plugins/auth/AuthenticationConfig.php @@ -61,13 +61,7 @@ class AuthenticationConfig extends AuthenticationPlugin */ public function authSetUser() { - // try to workaround PHP 5 session garbage collection which - // looks at the session file's last modified time - if (isset($_REQUEST['access_time'])) { - $_SESSION['last_access_time'] = time() - $_REQUEST['access_time']; - } else { - $_SESSION['last_access_time'] = time(); - } + $this->setSessionAccessTime(); return true; } diff --git a/libraries/plugins/auth/AuthenticationCookie.php b/libraries/plugins/auth/AuthenticationCookie.php index f953c4c150..0d37f4db32 100644 --- a/libraries/plugins/auth/AuthenticationCookie.php +++ b/libraries/plugins/auth/AuthenticationCookie.php @@ -450,11 +450,7 @@ class AuthenticationCookie extends AuthenticationPlugin // Avoid showing the password in phpinfo()'s output unset($GLOBALS['PHP_AUTH_PW']); unset($_SERVER['PHP_AUTH_PW']); - if (isset($_REQUEST['access_time'])) { - $_SESSION['last_access_time'] = time() - $_REQUEST['access_time']; - } else { - $_SESSION['last_access_time'] = time(); - } + $this->setSessionAccessTime(); } /** diff --git a/libraries/plugins/auth/AuthenticationHttp.php b/libraries/plugins/auth/AuthenticationHttp.php index 62223f88a0..3d33186478 100644 --- a/libraries/plugins/auth/AuthenticationHttp.php +++ b/libraries/plugins/auth/AuthenticationHttp.php @@ -228,9 +228,7 @@ class AuthenticationHttp extends AuthenticationPlugin unset($GLOBALS['PHP_AUTH_PW']); unset($_SERVER['PHP_AUTH_PW']); - // try to workaround PHP 5 session garbage collection which - // looks at the session file's last modified time - $_SESSION['last_access_time'] = time(); + $this->setSessionAccessTime(); return true; }