From 7d93aa6a100a65b622896346a825beabc2caaf74 Mon Sep 17 00:00:00 2001 From: Starbeamrainbowlabs Date: Tue, 28 Jul 2020 19:40:22 +0100 Subject: [PATCH] Overhaul the way we use setcookie() - Use SameSite=Strict to avoid issues in modern browsers & prevent session-stealing attacks - Use Secure when requests run over HTTPS by default to avoid downgrade-based session-stealing attacks - Add warning for PHP <= 7.2, as it doesn't support SameSite in setcookie(). --- Changelog.md | 1 + core/02-environment.php | 16 +++++++++------- core/05-functions.php | 40 ++++++++++++++++++++++++++++++++++++++- core/10-login.php | 2 +- modules/extra-sidebar.php | 4 ++-- peppermint.guiconfig.json | 3 ++- 6 files changed, 54 insertions(+), 12 deletions(-) diff --git a/Changelog.md b/Changelog.md index 393bf61..d42f7de 100644 --- a/Changelog.md +++ b/Changelog.md @@ -32,6 +32,7 @@ This file holds the changelog for Pepperminty Wiki. This is the master list of t - Don't worry, we've absorbed all the useful features (see above) - NOTE TO SELF: Don't forget to update wikimatrix.org when we next make a stable release! (if you are reading this in the release notes for a stable release, please get in touch) - Enabled horizontal resize handle on sidebar (but it doesn't persist yet) + - `SameSite=Strict` is now set on all cookies in PHP 7.3+ to comply with the [new samesite cookies rules](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite#SameSiteNone_requires_Secure). A warning is generated in PHP 7.2 and below = [please upgrade](https://www.php.net/supported-versions.php) to PHP 7.3+! (#200) ### Fixed - Squashed a warning when using the fenced code block syntax diff --git a/core/02-environment.php b/core/02-environment.php index 21f0504..eaf7b53 100644 --- a/core/02-environment.php +++ b/core/02-environment.php @@ -6,26 +6,28 @@ $commit = "{commit}"; /// Environment /// /** Holds information about the current request environment. */ $env = new stdClass(); -/** The action requested by the user. */ +/** The action requested by the user. @var string */ $env->action = $settings->defaultaction; -/** The page name requested by the remote client. */ +/** The page name requested by the remote client. @var string */ $env->page = ""; -/** The filename that the page is stored in. */ +/** The filename that the page is stored in. @var string */ $env->page_filename = ""; -/** Whether we are looking at a history revision. */ +/** Whether we are looking at a history revision or not. @var boolean */ $env->is_history_revision = false; /** An object holding history revision information for the current request */ $env->history = new stdClass(); -/** The revision number requested of the current page */ +/** The revision number requested of the current page @var int */ $env->history->revision_number = -1; /** The revision data object from the page index for the requested revision */ $env->history->revision_data = false; /** The user's name if they are logged in. Defaults to `$settings->anonymous_user_name` if the user isn't currently logged in. @var string */ $env->user = $settings->anonymous_user_name; -/** Whether the user is logged in */ +/** Whether the user is logged in @var boolean */ $env->is_logged_in = false; -/** Whether the user is an admin (moderator) @todo Refactor this to is_moderator, so that is_admin can be for the server owner. */ +/** Whether the user is an admin (moderator) @todo Refactor this to is_moderator, so that is_admin can be for the server owner. @var boolean */ $env->is_admin = false; +/** Whether the current request was made a secure connection or not. @var boolean */ +$env->is_secure = !empty( $s['HTTPS'] ) && $s['HTTPS'] !== 'off'; /** The currently logged in user's data. Please see $settings->users->username if you need to edit this - this is here for convenience :-) */ $env->user_data = new stdClass(); /** The data storage directory. Page filenames should be prefixed with this if you want their content. */ diff --git a/core/05-functions.php b/core/05-functions.php index 59ab21f..47e3538 100644 --- a/core/05-functions.php +++ b/core/05-functions.php @@ -9,8 +9,9 @@ */ function url_origin( $s = false, $use_forwarded_host = false ) { + global $env; if($s === false) $s = $_SERVER; - $ssl = ( ! empty( $s['HTTPS'] ) && $s['HTTPS'] == 'on' ); + $ssl = $env->is_secure; $sp = strtolower( $s['SERVER_PROTOCOL'] ); $protocol = substr( $sp, 0, strpos( $sp, '/' ) ) . ( ( $ssl ) ? 's' : '' ); $port = $s['SERVER_PORT']; @@ -841,3 +842,40 @@ function metrics2servertiming(stdClass $perfdata) : string { } return "foo, ".implode(", ", $result); } + +/** + * Sets a cookie on the client via the set-cookie header. + * Uses setcookie() under-the-hood. + * @param string $key The cookie name to set. + * @param string $value The cookie value to set. + * @param int $expires The expiry time to set on the cookie. + * @return void + */ +function send_cookie(string $key, $value, int $expires) : void { + global $env, $settings; + + $cookie_secure = true; + switch ($settings->cookie_secure) { + case "false": + $cookie_secure = false; + break; + case "auto": + default: + $cookie_secure = $env->is_secure; + break; + } + + if(version_compare(PHP_VERSION, "7.3.0") >= 0) { + // Phew! We're running PHP 7.3+, so we're ok to use the array syntax + setcookie($key, $value, [ + "expires" => $expires, + "secure" => $cookie_secure, + "httponly" => true, + "samesite" => "Strict" + ]); + } + else { + if(!$env->is_secure) error_log("[pepperminty_wiki/$settings->sitename] Warning: You are using a version of PHP that is less than 7.3. This is not recommended - as the samesite cookie flag can't be set in PHP 7.3-, and this is insecure - as it opens you to session stealing attacks. In addition, browsers have deprecated non-samesite cookies in insecure contexts. Please upgrade today!"); + setcookie($key, $value, $expires, "", "", $cookie_secure, true); + } +} diff --git a/core/10-login.php b/core/10-login.php index c440db0..1a763bd 100644 --- a/core/10-login.php +++ b/core/10-login.php @@ -2,7 +2,7 @@ if(!is_cli()) session_start(); // Make sure that the login cookie lasts beyond the end of the user's session -setcookie(session_name(), session_id(), time() + $settings->sessionlifetime, "", "", false, true); +send_cookie(session_name(), session_id(), time() + $settings->sessionlifetime); ///////// Login System ///////// // Clear expired sessions if(isset($_SESSION[$settings->sessionprefix . "-expiretime"]) and diff --git a/modules/extra-sidebar.php b/modules/extra-sidebar.php index 8c15df2..05f42ce 100644 --- a/modules/extra-sidebar.php +++ b/modules/extra-sidebar.php @@ -20,7 +20,7 @@ register_module([ { $show_sidebar = true; // Set a cookie to persist the display of the sidebar - setcookie("sidebar_show", "true", time() + (60 * 60 * 24 * 30)); + send_cookie("sidebar_show", "true", time() + (60 * 60 * 24 * 30)); } // Show the sidebar if the cookie is set @@ -33,7 +33,7 @@ register_module([ { $show_sidebar = false; unset($_COOKIE["sidebar_show"]); - setcookie("sidebar_show", null, time() - 3600); + send_cookie("sidebar_show", null, time() - 3600); } page_renderer::register_part_preprocessor(function(&$parts) use ($show_sidebar) { diff --git a/peppermint.guiconfig.json b/peppermint.guiconfig.json index d5807ce..999beda 100644 --- a/peppermint.guiconfig.json +++ b/peppermint.guiconfig.json @@ -97,7 +97,7 @@ "password_cost_time": { "type": "number", "description": "The desired number of milliseconds to delay by when hashing passwords. Pepperminty Wiki will automatically update the value of password_cost to take the length of time specified here. If you're using PASSWORD_ARGON2I, then the auto-update will be disabled.", "default": 350}, "password_cost_time_interval": { "type": "number", "description": "The interval, in seconds, at which the password cost should be recalculated. Set to -1 to disable. Default: 1 week", "default": 604800}, "password_cost_time_lastcheck": { "type": "number", "description": "Pseudo-setting used to keep track of the last recalculation of password_cost. Is updated with the current unix timestamp every time password_cost is recalculated.", "default": 0}, - "new_password_length": { "type": "number", "description": "The length of newly-generated passwords. This is currently used in the user table when creating new accounts.", "default": 32}, + "new_password_length": { "type": "number", "description": "The length of newly-generated passwords. This is currently used in the user table when creating new accounts.", "default": 32 }, "require_login_view": { "type": "checkbox", "description": "Whether to require that users login before they do anything else. Best used with the data_storage_dir option.", "default": false}, "readingtime_enabled": { "type": "checkbox", "description": "Whether to display the estimated reading time beneath the header of every wiki page.", "default": true }, "readingtime_language": { "type": "text", "description": "The language code to use when estimating the reading time. Possible values: en, ar, de, es, fi, fr, he, it, jw, nl, pl, pt, ru, sk, sv, tr, zh. Unfrotuantely adding multi-language support to the user interface is an absolutely massive undertaking that would take ages, as Peppermitny Wiki waasn't designed with that in mind :-/", "default": "en" }, @@ -262,6 +262,7 @@ "stats_update_processingtime": { "type": "number", "description": "The maximum number of milliseconds that should be spent at once calculating statistics. If some statistics couldn't fit within this limit, then they are scheduled and updated on the next page load. Note that this is a target only - if an individual statistic takes longer than this, then it won't be interrupted. Defaults to 100ms.", "default": 100}, "sessionprefix": { "type": "text", "description": "You shouldn't need to change this. The prefix that should be used in the names of the session variables. Defaults to \"auto\", which automatically generates this field. See the readme for more information.", "default": "auto" }, "sessionlifetime": { "type": "number", "description": "Again, you shouldn't need to change this under normal circumstances. This setting controls the lifetime of a login session. Defaults to 24 hours, but it may get cut off sooner depending on the underlying PHP session lifetime.", "default": 86400 }, + "cookie_secure": { "type": "text", "description": "Whether to set the 'Secure' flag on all cookies. This prevents cookies from being transmitted over an unencrypted connection. Default: auto (sets the flag if HTTPS is detected). Other possible values: false (the flag is never set), true (the flag will always be set, regardless of whether HTTPS is detected or not)", "default": "auto" }, "disable_peppermint_access_check": { "type": "checkbox", "description": "Disables the access check for peppermint.json on first-run. VERY DANGEROUS. Use only for development. Note that it's recommend to block access to peppermint.json for a reason - it contains your site secret and password hashes, so an attacker could do all sorts of nefarious things if it's left unblocked.", "default": false }, "css_theme_autoupdate_url": { "type": "url", "description": "A url that points to the css theme file to check for updates. If blank, then automatic updates are disabled.", "default": "" }, "css_theme_autoupdate_interval": { "type": "number", "description": "The interval, in seconds, that updates to the theme should be checked for. Defaults to every week. A value of -1 disables automatic updates.", "default": 604800 },