From ebd650c06c67a67da2a0d099f625b6a7ec62ab2b Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Sun, 22 Oct 2017 18:44:46 +0200 Subject: Refactor session token management Relates to https://github.com/shaarli/Shaarli/issues/324 Added: - `SessionManager` class to group session-related features - unit tests Changed: - `getToken()` -> `SessionManager->generateToken()` - `tokenOk()` -> `SessionManager->checkToken()` - inject a `$token` parameter to `PageBuilder`'s constructor Signed-off-by: VirtualTam --- application/PageBuilder.php | 6 +++-- application/SessionManager.php | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 application/SessionManager.php (limited to 'application') diff --git a/application/PageBuilder.php b/application/PageBuilder.php index af290671..468f144b 100644 --- a/application/PageBuilder.php +++ b/application/PageBuilder.php @@ -32,12 +32,14 @@ class PageBuilder * * @param ConfigManager $conf Configuration Manager instance (reference). * @param LinkDB $linkDB instance. + * @param string $token Session token */ - public function __construct(&$conf, $linkDB = null) + public function __construct(&$conf, $linkDB = null, $token = null) { $this->tpl = false; $this->conf = $conf; $this->linkDB = $linkDB; + $this->token = $token; } /** @@ -92,7 +94,7 @@ class PageBuilder $this->tpl->assign('showatom', $this->conf->get('feed.show_atom', true)); $this->tpl->assign('feed_type', $this->conf->get('feed.show_atom', true) !== false ? 'atom' : 'rss'); $this->tpl->assign('hide_timestamps', $this->conf->get('privacy.hide_timestamps', false)); - $this->tpl->assign('token', getToken($this->conf)); + $this->tpl->assign('token', $this->token); if ($this->linkDB !== null) { $this->tpl->assign('tags', $this->linkDB->linksCountPerTag()); diff --git a/application/SessionManager.php b/application/SessionManager.php new file mode 100644 index 00000000..2083df42 --- /dev/null +++ b/application/SessionManager.php @@ -0,0 +1,53 @@ +session = &$session; + $this->conf = &$conf; + } + + /** + * Generates a session token + * + * @return string token + */ + public function generateToken() + { + $token = sha1(uniqid('', true) .'_'. mt_rand() . $this->conf->get('credentials.salt')); + $this->session['tokens'][$token] = 1; + return $token; + } + + /** + * Checks the validity of a session token, and destroys it afterwards + * + * @param string $token The token to check + * + * @return bool true if the token is valid, else false + */ + public function checkToken($token) + { + if (! isset($this->session['tokens'][$token])) { + // the token is wrong, or has already been used + return false; + } + + // destroy the token to prevent future use + unset($this->session['tokens'][$token]); + return true; + } +} -- cgit v1.2.3 From fd7d84616d53486c3a276a42da869390e1d7f5eb Mon Sep 17 00:00:00 2001 From: VirtualTam Date: Sun, 22 Oct 2017 19:54:44 +0200 Subject: Move session ID check to SessionManager Relates to https://github.com/shaarli/Shaarli/issues/324 Changed: - `is_session_id_valid()` -> `SessionManager::checkId()` - update tests Signed-off-by: VirtualTam --- application/SessionManager.php | 30 ++++++++++++++++++++++++++++++ application/Utils.php | 30 ------------------------------ 2 files changed, 30 insertions(+), 30 deletions(-) (limited to 'application') diff --git a/application/SessionManager.php b/application/SessionManager.php index 2083df42..3aa4ddfc 100644 --- a/application/SessionManager.php +++ b/application/SessionManager.php @@ -50,4 +50,34 @@ class SessionManager unset($this->session['tokens'][$token]); return true; } + + /** + * Validate session ID to prevent Full Path Disclosure. + * + * See #298. + * The session ID's format depends on the hash algorithm set in PHP settings + * + * @param string $sessionId Session ID + * + * @return true if valid, false otherwise. + * + * @see http://php.net/manual/en/function.hash-algos.php + * @see http://php.net/manual/en/session.configuration.php + */ + public static function checkId($sessionId) + { + if (empty($sessionId)) { + return false; + } + + if (!$sessionId) { + return false; + } + + if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) { + return false; + } + + return true; + } } diff --git a/application/Utils.php b/application/Utils.php index 2f38a8de..97b12fcf 100644 --- a/application/Utils.php +++ b/application/Utils.php @@ -181,36 +181,6 @@ function generateLocation($referer, $host, $loopTerms = array()) return $finalReferer; } -/** - * Validate session ID to prevent Full Path Disclosure. - * - * See #298. - * The session ID's format depends on the hash algorithm set in PHP settings - * - * @param string $sessionId Session ID - * - * @return true if valid, false otherwise. - * - * @see http://php.net/manual/en/function.hash-algos.php - * @see http://php.net/manual/en/session.configuration.php - */ -function is_session_id_valid($sessionId) -{ - if (empty($sessionId)) { - return false; - } - - if (!$sessionId) { - return false; - } - - if (!preg_match('/^[a-zA-Z0-9,-]{2,128}$/', $sessionId)) { - return false; - } - - return true; -} - /** * Sniff browser language to set the locale automatically. * Note that is may not work on your server if the corresponding locale is not installed. -- cgit v1.2.3