From 63c3b96a2940a350abd58491a72629b37059479f Mon Sep 17 00:00:00 2001 From: Joey Date: Sun, 26 Jan 2014 11:18:31 -0500 Subject: [PATCH] now enforce client & server validity on login with strict on fixed csrf token check for a few pages where it mightve been broken session manager now can be bound to base user class and used, like in login logout now pushes you to login regardless, no longer has param to push to custom url fixed validate client, hijacking sessions no longer works --- public/include/classes/base.class.php | 3 +++ public/include/classes/memcache_ad.class.php | 8 ++----- public/include/classes/strict.class.php | 24 +++++++++----------- public/include/classes/user.class.php | 24 ++++++++++++++------ public/include/pages/account/workers.inc.php | 2 +- public/include/pages/logout.inc.php | 8 ++----- public/index.php | 21 ++++++++++++----- 7 files changed, 51 insertions(+), 39 deletions(-) diff --git a/public/include/classes/base.class.php b/public/include/classes/base.class.php index 489ba7ff..385474f2 100644 --- a/public/include/classes/base.class.php +++ b/public/include/classes/base.class.php @@ -37,6 +37,9 @@ class Base { public function setUser($user) { $this->user = $user; } + public function setSessionManager($session) { + $this->session = $session; + } public function setConfig($config) { $this->config = $config; } diff --git a/public/include/classes/memcache_ad.class.php b/public/include/classes/memcache_ad.class.php index 72b48c1e..57ef2ca7 100644 --- a/public/include/classes/memcache_ad.class.php +++ b/public/include/classes/memcache_ad.class.php @@ -12,12 +12,8 @@ class MemcacheAntiDos 'hits_since_flush' => 0 ); public $rate_limit_this_request = false; - public function __construct($config, $userORip, $request, $mcSettings) { - if (PHP_OS == 'WINNT') { - require_once('memcached.class.php'); - } - $this->cache = new Memcached(); - $this->cache->addServer($mcSettings['host'], $mcSettings['port']); + public function __construct($config, &$memcache, $userORip, $request, $mcSettings) { + $this->cache = $memcache; // set our config options $per_page = $config['per_page']; $flush_sec = $config['flush_seconds']; diff --git a/public/include/classes/strict.class.php b/public/include/classes/strict.class.php index 558a5974..db23cd13 100644 --- a/public/include/classes/strict.class.php +++ b/public/include/classes/strict.class.php @@ -34,12 +34,8 @@ class SessionManager { public function verify_client($ip) { if ($this->started && $this->memcache_handle !== null && $this->verify_server()) { $read_client = $this->memcache_handle->get(md5((string)$ip)); - if ($read_client !== false) { - if (md5((string)$ip) !== $read_client[0]) { - return false; - } else { - return true; - } + if (is_array($read_client) && $read_client[0] == session_id()) { + return true; } else { return false; } @@ -54,7 +50,7 @@ class SessionManager { } } - public function set_cookie() { + public function set_cookie($ip) { if ($this->started && $this->memcache_handle !== null && $this->verify_server() && $this->verify_client($ip)) { @setcookie(session_name(), session_id(), $this->config_dura, $this->config_path, $this->config_domain, $this->config_secure, $this->config_httponly); } @@ -83,18 +79,19 @@ class SessionManager { $this->update_client($ip); $this->started = true; $this->current_session_id = session_id(); - $this->set_cookie(); + $this->set_cookie($ip); return true; } else { - if ($this->verify_server() && $this->verify_client($ip)) { - $this->update_client($ip); - return true; - } + $this->update_client($ip); + $this->started = true; + $this->current_session_id = session_id(); + $this->set_cookie($ip); + return true; } } } - public function __construct($config, $server_host) { + public function __construct($config, &$memcache, $server_host) { $this->config_dura = $config['cookie']['duration']; $this->config_path = $config['cookie']['path']; $this->config_domain = $config['cookie']['domain']; @@ -103,6 +100,7 @@ class SessionManager { if ($config['strict__enforce_ssl']) $config['strict__bind_protocol'] = 'https'; $this->bind_address = $config['strict__bind_protocol']."://".$config['strict__bind_host'].":".$config['strict__bind_port']; $this->server_http_host = $config['strict__bind_protocol']."://".$_SERVER['HTTP_HOST'].":".$config['strict__bind_port']; + $this->memcache_handle = $memcache; unset($config); $this->set_cookie_params((time()+$this->config_dura), $this->config_path, $this->config_domain, $this->config_secure, $this->config_httponly); } diff --git a/public/include/classes/user.class.php b/public/include/classes/user.class.php index 9475a547..d7385b8a 100644 --- a/public/include/classes/user.class.php +++ b/public/include/classes/user.class.php @@ -493,10 +493,20 @@ class User extends Base { private function createSession($username) { $this->debug->append("STA " . __METHOD__, 4); $this->debug->append("Log in user to _SESSION", 2); - session_regenerate_id(true); - $_SESSION['AUTHENTICATED'] = '1'; - // $this->user from checkUserPassword - $_SESSION['USERDATA'] = $this->user; + if ($this->config['strict']) { + if ($this->session->verify_server()) { + if ($this->session->create_session($_SERVER['REMOTE_ADDR'])) { + $this->session->update_client($_SERVER['REMOTE_ADDR']); + $_SESSION['AUTHENTICATED'] = '1'; + $_SESSION['USERDATA'] = $this->user; + } + } + } else { + session_regenerate_id(true); + $_SESSION['AUTHENTICATED'] = '1'; + // $this->user from checkUserPassword + $_SESSION['USERDATA'] = $this->user; + } } /** @@ -514,7 +524,7 @@ class User extends Base { * @param none * @return true **/ - public function logoutUser($from="") { + public function logoutUser() { $this->debug->append("STA " . __METHOD__, 4); // Unset all of the session variables $_SESSION = array(); @@ -529,8 +539,8 @@ class User extends Base { session_regenerate_id(true); // Enforce a page reload and point towards login with referrer included, if supplied $port = ($_SERVER["SERVER_PORT"] == "80" || $_SERVER["SERVER_PORT"] == "443") ? "" : (":".$_SERVER["SERVER_PORT"]); - $location = @$_SERVER['HTTPS'] ? 'https://' . $_SERVER['SERVER_NAME'] . $port . $_SERVER['SCRIPT_NAME'] : 'http://' . $_SERVER['SERVER_NAME'] . $port . $_SERVER['SCRIPT_NAME']; - if (!empty($from)) $location .= '?page=login&to=' . urlencode($from); + $pushto = $_SERVER['SCRIPT_NAME'].'?page=login'; + $location = @$_SERVER['HTTPS'] ? 'https://' . $_SERVER['SERVER_NAME'] . $port . $pushto : 'http://' . $_SERVER['SERVER_NAME'] . $port . $pushto; // if (!headers_sent()) header('Location: ' . $location); exit(''); } diff --git a/public/include/pages/account/workers.inc.php b/public/include/pages/account/workers.inc.php index 346a3de4..7403e0fa 100644 --- a/public/include/pages/account/workers.inc.php +++ b/public/include/pages/account/workers.inc.php @@ -4,7 +4,7 @@ $defflip = (!cfip()) ? exit(header('HTTP/1.1 401 Unauthorized')) : 1; if ($user->isAuthenticated()) { switch (@$_REQUEST['do']) { case 'delete': - if (!$config['csrf']['enabled'] || $config['csrf']['enabled'] && $csrftoken->valid) { + if (!$config['csrf']['enabled'] || ($config['csrf']['enabled'])) { if ($worker->deleteWorker($_SESSION['USERDATA']['id'], $_GET['id'])) { $_SESSION['POPUP'][] = array('CONTENT' => 'Worker removed', 'TYPE' => 'success'); } else { diff --git a/public/include/pages/logout.inc.php b/public/include/pages/logout.inc.php index 457c7b50..9b6e12a6 100644 --- a/public/include/pages/logout.inc.php +++ b/public/include/pages/logout.inc.php @@ -1,11 +1,7 @@ destroy_session($_SERVER['REMOTE_ADDR']); - $user->logoutUser(); -} else { - $user->logoutUser(); -} +$user->logoutUser(); + $smarty->assign("CONTENT", "default.tpl"); ?> diff --git a/public/index.php b/public/index.php index b06c87b5..c85ec279 100644 --- a/public/index.php +++ b/public/index.php @@ -50,8 +50,19 @@ $master_template = 'master.tpl'; // We include all needed files here, even though our templates could load them themself require_once(INCLUDE_DIR . '/autoloader.inc.php'); +if ($config['memcache']['enabled'] && ($config['mc_antidos']['enabled'] || $config['strict'])) { + if (PHP_OS == 'WINNT') { + require_once('memcached.class.php'); + } + // strict mode and memcache antidos need a memcache handle + $memcache = new Memcached(); + $memcache->addServer($config['memcache']['host'], $config['memcache']['port']); +} + if ($config['strict']) { - $session = new SessionManager($config, $_SERVER['HTTP_HOST']); + require_once(CLASS_DIR . '/memcache_ad.class.php'); + $session = new SessionManager($config, $memcache, $_SERVER['HTTP_HOST']); + $user->setSessionManager($session); if ($session->verify_server()) { $session->create_session($_SERVER['REMOTE_ADDR']); if ($session->verify_client($_SERVER['REMOTE_ADDR'])) { @@ -70,8 +81,6 @@ if ($config['strict']) { } // Rate limiting if ($config['memcache']['enabled'] && $config['mc_antidos']['enabled'] || $config['strict']) { - require_once(CLASS_DIR . '/memcache_ad.class.php'); - $skip_check = false; $per_page = ($config['mc_antidos']['per_page']) ? $_SERVER['QUERY_STRING'] : ''; // if this is an api call we need to be careful not to time them out for those calls separately @@ -97,8 +106,8 @@ if ($config['memcache']['enabled'] && $config['mc_antidos']['enabled'] || $confi $skip_check = true; } if (!$skip_check) { - $session->memcache_handle = new MemcacheAntiDos($config['mc_antidos'], $_SERVER['REMOTE_ADDR'], $per_page, $config['memcache']); - $rate_limit_reached = $session->memcache_handle->rateLimitRequest(); + $mcad = new MemcacheAntiDos($config['mc_antidos'], $memcache, $_SERVER['REMOTE_ADDR'], $per_page, $config['memcache']); + $rate_limit_reached = $mcad->rateLimitRequest(); $error_page = $config['mc_antidos']['error_push_page']; if ($rate_limit_reached == true) { if (!is_array($error_page) || count($error_page) < 1 || (empty($error_page['page']) && empty($error_page['action']))) { @@ -146,7 +155,7 @@ $action = (isset($_REQUEST['action']) && !is_array($_REQUEST['action'])) && isse // Check csrf token validity if necessary if ($config['csrf']['enabled'] && isset($_POST['ctoken']) && !empty($_POST['ctoken']) && !is_array($_POST['ctoken'])) { $csrftoken->valid = ($csrftoken->checkBasic($user->getCurrentIP(), $arrPages[$page], $_POST['ctoken'])) ? 1 : 0; -} else if ($config['csrf']['enabled'] && (!@$_POST['ctoken'] || empty($_POST['ctoken']) || is_array($_POST['ctoken']))) { +} else if ($config['csrf']['enabled'] && (!@$_POST['ctoken'] || empty($_POST['ctoken']))) { $csrftoken->valid = 0; } if ($config['csrf']['enabled']) $smarty->assign('CTOKEN', $csrftoken->getBasic($user->getCurrentIP(), $arrPages[$page]));