From a8c80c528189e834a7759376c0f3970dec213e1c Mon Sep 17 00:00:00 2001
From: Jan-Hendrik Willms <tleilax+studip@gmail.com>
Date: Wed, 3 Jan 2024 19:22:48 +0000
Subject: [PATCH] fixes #3204
Closes #3204
Merge request studip/studip!2170
---
lib/classes/CSRFProtection.php | 40 ++++++++++++++---
lib/classes/StudipCoreFormat.php | 3 +-
lib/classes/StudipPDO.class.php | 1 +
lib/classes/Visibility.php | 4 +-
lib/models/CronjobSchedule.class.php | 2 +-
lib/models/SimpleORMap.class.php | 4 +-
lib/visual.inc.php | 6 +--
tests/unit/lib/classes/CsrfProtectionTest.php | 44 ++++++++++---------
tests/unit/lib/classes/MigrationTest.php | 2 +-
tests/unit/lib/classes/TextFormatTest.php | 1 +
10 files changed, 69 insertions(+), 38 deletions(-)
diff --git a/lib/classes/CSRFProtection.php b/lib/classes/CSRFProtection.php
index 440919efe92..56d6e4cc836 100644
--- a/lib/classes/CSRFProtection.php
+++ b/lib/classes/CSRFProtection.php
@@ -44,6 +44,35 @@ class CSRFProtection
const AJAX_TOKEN = 'HTTP_X_CSRF_TOKEN';
+ protected static $storage = null;
+
+ /**
+ * Set a storage to use.
+ *
+ * @param $storage
+ */
+ public static function setStorage(&$storage): void
+ {
+ self::$storage = &$storage;
+ }
+
+ /**
+ * Returns a reference to the used storage.
+ *
+ * @return array|null
+ */
+ public static function &getStorage()
+ {
+ if (!isset(self::$storage)) {
+ // w/o a session, throw an exception since we cannot use it
+ if (session_id() === '') {
+ throw new SessionRequiredException();
+ }
+
+ self::$storage = $_SESSION;
+ }
+ return self::$storage;
+ }
/**
* This checks the request and throws an InvalidSecurityTokenException if
@@ -118,17 +147,14 @@ class CSRFProtection
*/
public static function token()
{
- // w/o a session, throw an exception
- if (session_id() === '') {
- throw new SessionRequiredException();
- }
+ $storage = &self::getStorage();
// create a token, if there is none
- if (!isset($_SESSION[self::TOKEN])) {
- $_SESSION[self::TOKEN] = base64_encode(random_bytes(32));
+ if (!isset($storage[self::TOKEN])) {
+ $storage[self::TOKEN] = base64_encode(random_bytes(32));
}
- return $_SESSION[self::TOKEN];
+ return $storage[self::TOKEN];
}
/**
diff --git a/lib/classes/StudipCoreFormat.php b/lib/classes/StudipCoreFormat.php
index 4d4b14b855f..24c4ddcdf62 100644
--- a/lib/classes/StudipCoreFormat.php
+++ b/lib/classes/StudipCoreFormat.php
@@ -515,8 +515,6 @@ class StudipCoreFormat extends TextFormat
$email = $matches[2];
$domain = $matches[3];
- $intern = $domain === $_SERVER['HTTP_HOST'];
-
return sprintf('<a href="mailto:%1$s">%2$s</a>',
$email,
$link_text
@@ -567,6 +565,7 @@ class StudipCoreFormat extends TextFormat
$intern = false;
if (
in_array($pu['scheme'], ['http', 'https'])
+ && isset($_SERVER['HTTP_HOST'])
&& (
!isset($pu['host'])
|| $pu['host'] === $_SERVER['HTTP_HOST']
diff --git a/lib/classes/StudipPDO.class.php b/lib/classes/StudipPDO.class.php
index 3fe07188efb..08012b10545 100644
--- a/lib/classes/StudipPDO.class.php
+++ b/lib/classes/StudipPDO.class.php
@@ -83,6 +83,7 @@ class StudipPDO extends PDO
// split string into parts at quotes and backslash
$parts = preg_split('/([\\\\"\'])/', $statement, -1, PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE);
$result = '';
+ $quote_chr = null;
for ($part = current($parts); $part !== false; $part = next($parts)) {
// inside quotes, "" is ", '' is ' and \x is x
diff --git a/lib/classes/Visibility.php b/lib/classes/Visibility.php
index ce191d2ab8f..1b872301152 100644
--- a/lib/classes/Visibility.php
+++ b/lib/classes/Visibility.php
@@ -500,8 +500,8 @@ class Visibility
*/
private static function getUser(&$user)
{
- if ($user == null) {
- $user = $GLOBALS['user']->user_id;
+ if (!$user && User::findCurrent()) {
+ $user = User::findCurrent()->id;
}
}
diff --git a/lib/models/CronjobSchedule.class.php b/lib/models/CronjobSchedule.class.php
index 6fcb68ff965..4304be17179 100644
--- a/lib/models/CronjobSchedule.class.php
+++ b/lib/models/CronjobSchedule.class.php
@@ -130,7 +130,7 @@ class CronjobSchedule extends SimpleORMap
}
if (in_array($type, ['after_initialize', 'after_store']) && is_string($this->parameters)) {
$parameters = json_decode($this->parameters, true) ?: [];
- if ($this->task->valid) {
+ if ($this->task && $this->task->valid) {
$default_parameters = $this->task->extractDefaultParameters();
foreach ($default_parameters as $key => $value) {
if (!isset($parameters[$key])) {
diff --git a/lib/models/SimpleORMap.class.php b/lib/models/SimpleORMap.class.php
index 3ff4cc08328..8f951b4abc3 100644
--- a/lib/models/SimpleORMap.class.php
+++ b/lib/models/SimpleORMap.class.php
@@ -863,7 +863,7 @@ class SimpleORMap implements ArrayAccess, Countable, IteratorAggregate
if (is_array($id_or_object)) {
$pk = static::pk();
$key_values = [];
- foreach($pk as $key) {
+ foreach ($pk as $key) {
if (array_key_exists($key, $id_or_object)) {
$key_values[] = $id_or_object[$key];
}
@@ -874,6 +874,8 @@ class SimpleORMap implements ArrayAccess, Countable, IteratorAggregate
} else {
$id = $key_values;
}
+ } else {
+ $id = null;
}
} else {
$id = $id_or_object;
diff --git a/lib/visual.inc.php b/lib/visual.inc.php
index 30f99497519..5cef921b16d 100644
--- a/lib/visual.inc.php
+++ b/lib/visual.inc.php
@@ -294,9 +294,9 @@ function isLinkIntern($url) {
return true;
}
- return in_array($pum['scheme'] ?? null, ['https', 'http', NULL], true)
- && in_array($pum['host'] ?? null, [$_SERVER['SERVER_NAME'], NULL], true)
- && in_array($pum['port'] ?? null, [$_SERVER['SERVER_PORT'], NULL], true)
+ return in_array($pum['scheme'] ?? null, ['https', 'http', null], true)
+ && in_array($pum['host'] ?? null, [$_SERVER['SERVER_NAME'] ?? null, null], true)
+ && in_array($pum['port'] ?? null, [$_SERVER['SERVER_PORT'] ?? null, null], true)
&& mb_strpos($pum['path'] ?? '', $GLOBALS['CANONICAL_RELATIVE_PATH_STUDIP']) === 0;
}
diff --git a/tests/unit/lib/classes/CsrfProtectionTest.php b/tests/unit/lib/classes/CsrfProtectionTest.php
index 1a74598188b..69bc7a61960 100644
--- a/tests/unit/lib/classes/CsrfProtectionTest.php
+++ b/tests/unit/lib/classes/CsrfProtectionTest.php
@@ -12,27 +12,18 @@
class CSRFProtectionTokenTest extends \Codeception\Test\Unit
{
- private $original_session;
+ use CsrfProtectionSessionTrait;
function setUp(): void
{
- if (session_id() === '') {
- session_id("test-session");
- }
- $this->original_session = $_SESSION;
- $_SESSION = [];
- }
-
- function tearDown(): void
- {
- $_SESSION = $this->original_session;
+ $this->initializeTokenStorage();
}
function testTokenGeneration()
{
- $this->assertEquals(sizeof($_SESSION), 0);
+ $this->assertEquals(count($this->storage), 0);
CSRFProtection::token();
- $this->assertEquals(sizeof($_SESSION), 1);
+ $this->assertEquals(count($this->storage), 1);
}
function testTokenIdentity()
@@ -44,7 +35,7 @@ class CSRFProtectionTokenTest extends \Codeception\Test\Unit
{
$token1 = CSRFProtection::token();
- $_SESSION = [];
+ $this->storage = [];
$token2 = CSRFProtection::token();
@@ -66,16 +57,17 @@ class CSRFProtectionTokenTest extends \Codeception\Test\Unit
class CSRFRequestTest extends \Codeception\Test\Unit
{
+ use CsrfProtectionSessionTrait;
+
private $original_state;
private $token;
function setUp(): void
{
- if (session_id() === '') {
- session_id("test-session");
- }
- $this->original_state = [$_SESSION, $_POST, $_SERVER];
- $_SESSION = [];
+ $this->initializeTokenStorage();
+
+ $this->original_state = [$_POST, $_SERVER];
+
$_POST = [];
$this->token = CSRFProtection::token();
$_SERVER['HTTP_X_REQUESTED_WITH'] = null;
@@ -83,7 +75,7 @@ class CSRFRequestTest extends \Codeception\Test\Unit
function tearDown(): void
{
- list($_SESSION, $_POST, $_SERVER) = $this->original_state;
+ [$_POST, $_SERVER] = $this->original_state;
}
function testInvalidUnsafeRequest()
@@ -96,7 +88,7 @@ class CSRFRequestTest extends \Codeception\Test\Unit
function testValidUnsafeRequest()
{
$_SERVER['REQUEST_METHOD'] = 'POST';
- $_POST['security_token'] = $this->token;
+ $_POST[CSRFProtection::TOKEN] = $this->token;
CSRFProtection::verifyUnsafeRequest();
$this->assertTrue(true);
}
@@ -134,3 +126,13 @@ class CSRFRequestTest extends \Codeception\Test\Unit
$this->assertTrue(true);
}
}
+
+trait CsrfProtectionSessionTrait
+{
+ protected $storage = [];
+
+ protected function initializeTokenStorage()
+ {
+ CSRFProtection::setStorage($this->storage);
+ }
+}
diff --git a/tests/unit/lib/classes/MigrationTest.php b/tests/unit/lib/classes/MigrationTest.php
index dd78ac7ed7d..444ebf2fe8e 100644
--- a/tests/unit/lib/classes/MigrationTest.php
+++ b/tests/unit/lib/classes/MigrationTest.php
@@ -58,7 +58,7 @@ class MigrationTest extends \Codeception\Test\Unit
public function get($branch = 0)
{
- return $this->versions[$branch];
+ return $this->versions[$branch] ?? 0;
}
public function set($version, $branch = 0)
diff --git a/tests/unit/lib/classes/TextFormatTest.php b/tests/unit/lib/classes/TextFormatTest.php
index 2174bb9dd56..0d5007621ba 100644
--- a/tests/unit/lib/classes/TextFormatTest.php
+++ b/tests/unit/lib/classes/TextFormatTest.php
@@ -101,6 +101,7 @@ function markupList($markup, $matches)
$rows = explode("\n", rtrim($matches[0]));
$indent = 0;
+ $result = '';
foreach ($rows as $row) {
list($level, $text) = explode(' ', $row, 2);
$level = mb_strlen($level);
--
GitLab