From 56194034f83fcdcf9600b9014f03bcdbeee09dd6 Mon Sep 17 00:00:00 2001
From: Dave Mc Nicoll <info@mcnd.ca>
Date: Thu, 23 Jan 2020 13:45:19 -0500
Subject: [PATCH] - Fixed a bug within cookie get/set - Added a secure hash to
 cookie.

---
 src/Cookie.php                                | 65 +++++++++++--------
 .../CookieInvalidSecureHashException.php      |  5 ++
 src/Session.php                               | 33 ++++++++--
 src/SessionMiddleware.php                     | 11 +++-
 4 files changed, 78 insertions(+), 36 deletions(-)
 create mode 100644 src/Exception/CookieInvalidSecureHashException.php

diff --git a/src/Cookie.php b/src/Cookie.php
index 1bcf117..8139518 100644
--- a/src/Cookie.php
+++ b/src/Cookie.php
@@ -5,7 +5,6 @@ namespace Storage;
 class Cookie {
 
     protected array $options = [
-        'name'     => '',
         'expires'  => 0,
         'path'     => null,
         'domain'   => null,
@@ -14,11 +13,9 @@ class Cookie {
         'samesite' => "Strict",
     ];
 
-    protected array $options;
+    protected ? string $secureHash;
 
-    protected string $secureHash;
-
-    public function __construct(array $options, string $secureHash) {
+    public function __construct(array $options, ? string $secureHash) {
         $this->options = $options;
         $this->secureHash = $secureHash;
     }
@@ -34,7 +31,7 @@ class Cookie {
      * @param   boolean  HTTP only (requires PHP 5.2 or higher)
      * @return  boolean
      */
-    public static function set(
+    public function set(
         ?string $name,
         ?string $value = null,
         ?int $expires = null,
@@ -49,7 +46,7 @@ class Cookie {
             return false;
         }
 
-        $expire = ($expire == 0) ? 0 : time() + (int) $expire;
+        $expires = ( $expires === 0 ) ? 0 : time() + (int) $expires;
 
         $_COOKIE[$name] = $value;
 
@@ -62,6 +59,10 @@ class Cookie {
             'samesite' => $this->options['samesite'] ?? ( $samesite ?: "" ),
         ];
 
+        if ( $value ) {
+            $value = sha1($this->secureHash . $value . $this->secureHash) . "|$value";
+        }
+
         return $raw ? setrawcookie($name, $value ?: "", $options) : setcookie($name, $value ?: "", $options);
     }
 
@@ -72,9 +73,28 @@ class Cookie {
      * @param   mixed    default value
      * @return  string
      */
-    public static function get(string $key, $default = null)
+    public function get(string $key, $default = null)
     {
-        return array_key_exists($key, $_COOKIE) ? $_COOKIE[$key] : $default;
+        if ( ! $this->has($key) ) {
+            return $default;
+        }
+
+        if ( $this->secureHash ) {
+            list($hash, $value) = explode('|', $_COOKIE[$key], 2);
+
+            if (! $this->isSecure($hash, $value)) {
+                throw new Exception\CookieInvalidSecureHashException();
+            }
+
+            return $value;
+        }
+
+        return $_COOKIE[$key];
+    }
+
+    public function has($key) : bool
+    {
+        return array_key_exists($key, $_COOKIE ?? []);
     }
 
     /**
@@ -84,33 +104,26 @@ class Cookie {
      * @param   string   URL domain
      * @return  boolean
      */
-    public static function delete(string $name, ?string $path = null, ?string $domain = null) {
-        if ( ! $this->__isset($name) ) {
+    public function delete(string $name, ?string $path = null, ?string $domain = null) {
+        if ( ! $this->has($name) ) {
             return false;
         }
 
         unset( $_COOKIE[$name] );
 
-        return static::set($name, '', -86400, $path ?: ( $this->options['path'] ?? "" ), $domain ?: ( $this->options['domain'] ?? "" ), $this->options['secure'] ?? false, $this->options['httponly'] ?? false);
+        return $this->set($name, '', -86400, $path ?: ( $this->options['path'] ?? "" ), $domain ?: ( $this->options['domain'] ?? "" ), $this->options['secure'] ?? false, $this->options['httponly'] ?? false);
     }
 
-    public function __set($key, $value)
+    public function isSecure($hash, $value) : bool
     {
-        return static::set($key, $value);
+        return sha1($this->secureHash . $value . $this->secureHash) === $hash;
     }
 
-    public function __get($key)
+    public function __invoke(...$arguments)
     {
-        return static::get($key);
-    }
-
-    public function __isset($key)
-    {
-        return array_key_exists($key, $_SESSION);
-    }
-
-    public function __unset($key)
-    {
-        $this->delete($key);
+        switch(count($arguments)) {
+            case 1: return $this->get(...$arguments);
+            case 2: return $this->set(...$arguments);
+        }
     }
 }
diff --git a/src/Exception/CookieInvalidSecureHashException.php b/src/Exception/CookieInvalidSecureHashException.php
new file mode 100644
index 0000000..53187dd
--- /dev/null
+++ b/src/Exception/CookieInvalidSecureHashException.php
@@ -0,0 +1,5 @@
+<?php
+
+namespace Stoarge\Exception;
+
+class CookieInvalidSecureHashException extends \Exception {}
diff --git a/src/Session.php b/src/Session.php
index bba3b0a..1c9c7cf 100644
--- a/src/Session.php
+++ b/src/Session.php
@@ -2,11 +2,15 @@
 
 namespace Storage;
 
+use session_name, session_id, session_start, session_destroy, session_save_path,
+    session_regenerate_id, session_cache_limiter, session_get_cookie_params,
+    session_set_cookie_params, session_status, time, array_key_exists;
+
 class Session
 {
     public static function get($key, $default = null)
     {
-        return array_key_exists($key, $_SESSION) ? $_SESSION[$key] : $default;
+        return static::has($key) ? $_SESSION[$key] : $default;
     }
 
     public static function set($key, $value)
@@ -19,13 +23,20 @@ class Session
         unset($_SESSION[$key]);
     }
 
-    public static function clearAll()
+    public static function has($key) : bool
+    {
+        return array_key_exists($key, $_SESSION ?? []);
+    }
+
+    public static function clearAll() : void
     {
         $_SESSION = [];
     }
 
-    public static function start(array $options, Cookie $cookie)
+    public static function start(array $options, Cookie $cookie, string $savePath) : void
     {
+        session_save_path($savePath);
+
         if ( session_status() === PHP_SESSION_ACTIVE ) {
             return;
         }
@@ -33,7 +44,7 @@ class Session
         $params = session_get_cookie_params();
 
         $params = [
-            'expires'  => $options['lifetime'] ?? $params['lifetime'],
+            'lifetime' => $options['lifetime'] ?? $params['lifetime'],
             'path'     => $options['path']     ?? $params['path'],
             'domain'   => $options['domain']   ?? $params['domain'],
             'secure'   => $options['secure']   ?? $params['secure'],
@@ -53,7 +64,7 @@ class Session
         session_start();
 
         # Reset timeout after session started
-        $cookie->set(session_name(), session_id(), time() + $params['expires'], $params['path'], $params['domain'], $params['secure'], $params['httponly'], $params['samesite']);
+        $cookie->set(session_name(), session_id(), time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly'], $params['samesite']);
     }
 
     public static function regenerate()
@@ -65,10 +76,10 @@ class Session
 
     public static function destroy()
     {
-        $this->clearAll();
+        static::clearAll();
 
         if ( ini_get("session.use_cookies") ) {
-            $cookie->delete(session_name(), $params['path'] ?? "", $params['domain'] ?? "");
+            ( new Cookie([], null) )->delete(session_name(), $params['path'] ?? "", $params['domain'] ?? "");
         }
 
         if ( session_status() === PHP_SESSION_ACTIVE ) {
@@ -76,6 +87,14 @@ class Session
         }
     }
 
+    public function __invoke(...$arguments)
+    {
+        switch(count($arguments)) {
+            case 1: return static::get(...$arguments);
+            case 2: return static::set(...$arguments);
+        }
+    }
+
     public function __set($key, $value)
     {
         return static::set($key, $value);
diff --git a/src/SessionMiddleware.php b/src/SessionMiddleware.php
index 012f949..c394970 100644
--- a/src/SessionMiddleware.php
+++ b/src/SessionMiddleware.php
@@ -16,13 +16,15 @@ class SessionMiddleware implements MiddlewareInterface
         'domain' => null,
         'secure' => false,
         'httponly' => true,
-        'lifetime' => 28800,  # A day of work ~ 7 hours
+        'lifetime' => 28800,  # A day of work ~ 8 hours
         'samesite' => "",
     ];
 
     protected Cookie $cookie;
 
-    public function __construct(Cookie $cookie, array $options = [])
+    protected string $savePath;
+
+    public function __construct(Cookie $cookie, array $options = [], ?string $savePath = null)
     {
         $this->cookie = $cookie;
 
@@ -32,6 +34,8 @@ class SessionMiddleware implements MiddlewareInterface
         }
 
         $this->options = array_merge($this->options, $options);
+
+        $this->savePath = $savePath ?? sys_get_temp_dir();
     }
 
     /**
@@ -43,7 +47,8 @@ class SessionMiddleware implements MiddlewareInterface
      */
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler) : ResponseInterface
     {
-        Session::start($this->options, $this->cookie);
+        Session::start($this->options, $this->cookie, $this->savePath);
+
         return $handler->handle($request);
     }
 }