From 13df539811462ba8bff007cca93c0f51d827fd2c Mon Sep 17 00:00:00 2001
From: Dave MacFarlane <driusan@mcin.ca>
Date: Thu, 19 Aug 2021 13:21:33 -0400
Subject: [PATCH] [Security] Fix SameSite cookie CSRF protection (#7539)

1. The session variable that indicates to use it was set to "true".
   However, it should be a string indicating the value to use (either
   "strict" or "lax"). See: https://www.php.net/manual/en/session.configuration.php#ini.session.cookie-samesite
2. The use_strict_mode setting (See: https://www.php.net/manual/en/session.configuration.php#ini.session.use-strict-mode)
   in PHP is not enabled by default. This means that, even when logging out, the
   session_destroy/session_start procedure reuses the old cookie and does not set a new cookie with
   the proper samesite flag. All the PHP documentation I've seen says use_strict_mode should always
   be enabled, but defaults to disabled. This explicitly overrides the php.ini setting (default false)
   to set it at the beginning of the code, ensuring that NDB_Client properly generates a new session cookie.
---
 htdocs/AjaxHelper.php              | 1 +
 htdocs/index.php                   | 7 +++++++
 htdocs/survey.php                  | 2 ++
 php/libraries/NDB_Client.class.inc | 4 ++--
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/htdocs/AjaxHelper.php b/htdocs/AjaxHelper.php
index 765f2a94668..2c9e238d9b4 100644
--- a/htdocs/AjaxHelper.php
+++ b/htdocs/AjaxHelper.php
@@ -31,6 +31,7 @@
     __DIR__ . "/../project/libraries:" .
     __DIR__ . "/../php/libraries"
 );
+ini_set('session.use_strict_mode', '1');
 
 require_once __DIR__ . "/../vendor/autoload.php";
 // Ensures the user is logged in, and parses the config file.
diff --git a/htdocs/index.php b/htdocs/index.php
index 925b1f029fe..3c8fd2b9d41 100644
--- a/htdocs/index.php
+++ b/htdocs/index.php
@@ -22,6 +22,13 @@
 // to be done before NDB_Client starts the PHP session.)
 session_cache_limiter("");
 
+// PHP documentation says this should always be enabled for session security.
+// PHP documentation says this is disabled by default.
+// Explicitly enable it.
+// phpcs:ignore
+// See: https://www.php.net/manual/en/session.configuration.php#ini.session.use-strict-mode
+ini_set('session.use_strict_mode', '1');
+
 // FIXME: The code in NDB_Client should mostly be replaced by middleware.
 $client = new \NDB_Client;
 $client->initialize();
diff --git a/htdocs/survey.php b/htdocs/survey.php
index 4baac3309fc..9e1225776a0 100644
--- a/htdocs/survey.php
+++ b/htdocs/survey.php
@@ -16,6 +16,8 @@
  */
 set_include_path(get_include_path().":../project/libraries:../php/libraries:");
 ini_set('default_charset', 'utf-8');
+ini_set('session.use_strict_mode', '1');
+
 require_once __DIR__ . "/../vendor/autoload.php";
 require_once 'NDB_Config.class.inc';
 require_once 'Smarty_hook.class.inc';
diff --git a/php/libraries/NDB_Client.class.inc b/php/libraries/NDB_Client.class.inc
index 714545d000f..621ec496408 100644
--- a/php/libraries/NDB_Client.class.inc
+++ b/php/libraries/NDB_Client.class.inc
@@ -132,8 +132,8 @@ class NDB_Client
             . $config_additions
         );
         // start php session
-        $sessionOptions = array('cookie_httponly' => true);
-        $sessionOptions['cookie_samesite'] = true;
+        $sessionOptions = ['cookie_httponly' => true];
+        $sessionOptions['cookie_samesite'] = "Strict";
 
         // API Detect
         if (strpos(