Skip to content

Commit 9a07c35

Browse files
simonLeary42simonLeary42
committed
copilot 1st draft
remove form helpers remove constants remove magic array_key_exists WIP single use tokens remove normal token in favor of single use tokens rename function better logging fixup require refactor add inputs to all forms validate in all pages remove file remove from init remove from ajax fixup tests init csrf_tokens array unique sessions for test cases automatically generate csrf tokens for http_post in testing add missing imports don't overwrite tokens validate in POST in header.php remove bad test set csrf_tokens array even if unauthenticated add session cleanup write session data to filesystem immediately on cleanup rename config option clear should leave an empty array must session_start before accessing $_SESSION change test dont validate twice in header.php update test case to check for double verification
1 parent 126ec72 commit 9a07c35

File tree

20 files changed

+243
-15
lines changed

20 files changed

+243
-15
lines changed

‎defaults/config.ini.default‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ enable_verbose_error_log = true ; internal use only
1818
enable_redirect_message = true ; internal use only
1919
enable_exception_handler = true ; internal use only
2020
enable_error_handler = true ; internal use only
21+
session_cleanup_idle_seconds = 1800 ; how long a session must be idle before messages and CSRF tokens are cleared
2122

2223
[ldap]
2324
uri = "ldap://identity" ; URI of remote LDAP server

‎resources/autoload.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
require_once __DIR__ . "/lib/UnityWebhook.php";
2525
require_once __DIR__ . "/lib/UnityGithub.php";
2626
require_once __DIR__ . "/lib/utils.php";
27+
require_once __DIR__ . "/lib/CSRFToken.php";
2728
require_once __DIR__ . "/lib/exceptions/NoDieException.php";
2829
require_once __DIR__ . "/lib/exceptions/SSOException.php";
2930
require_once __DIR__ . "/lib/exceptions/ArrayKeyException.php";

‎resources/init.php‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
set_error_handler(["UnityWebPortal\lib\UnityHTTPD", "errorHandler"]);
2222
}
2323

24-
session_start();
25-
2624
if (isset($GLOBALS["ldapconn"])) {
2725
$LDAP = $GLOBALS["ldapconn"];
2826
} else {
@@ -34,10 +32,24 @@
3432
$WEBHOOK = new UnityWebhook();
3533
$GITHUB = new UnityGithub();
3634

35+
session_start();
36+
// https://stackoverflow.com/a/1270960/18696276
37+
if (time() - ($_SESSION["LAST_ACTIVITY"] ?? 0) > CONFIG["site"]["session_cleanup_idle_seconds"]) {
38+
$_SESSION["csrf_tokens"] = [];
39+
$_SESSION["messages"] = [];
40+
session_write_close();
41+
session_start();
42+
}
43+
$_SESSION["LAST_ACTIVITY"] = time();
44+
3745
if (!array_key_exists("messages", $_SESSION)) {
3846
$_SESSION["messages"] = [];
3947
}
4048

49+
if (!array_key_exists("csrf_tokens", $_SESSION)) {
50+
$_SESSION["csrf_tokens"] = [];
51+
}
52+
4153
if (isset($_SERVER["REMOTE_USER"])) {
4254
// Check if SSO is enabled on this page
4355
$SSO = UnitySSO::getSSO();

‎resources/lib/CSRFToken.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
namespace UnityWebPortal\lib;
3+
4+
class CSRFToken
5+
{
6+
private static function ensureSessionCSRFTokensSanity(): void
7+
{
8+
if (!isset($_SESSION)) {
9+
throw new \RuntimeException("Session is not started. Call session_start() first.");
10+
}
11+
if (!array_key_exists("csrf_tokens", $_SESSION)) {
12+
UnityHTTPD::errorLog(
13+
"invalid session",
14+
'$_SESSION has no array key "csrf_tokens"',
15+
data: ['$_SESSION' => $_SESSION],
16+
);
17+
$_SESSION["csrf_tokens"] = [];
18+
}
19+
if (!is_array($_SESSION["csrf_tokens"])) {
20+
UnityHTTPD::errorLog(
21+
"invalid session",
22+
'$_SESSION["csrf_tokens"] is not an array',
23+
data: ['$_SESSION' => $_SESSION],
24+
);
25+
$_SESSION["csrf_tokens"] = [];
26+
}
27+
}
28+
29+
public static function generate(): string
30+
{
31+
self::ensureSessionCSRFTokensSanity();
32+
$token = bin2hex(random_bytes(32));
33+
$_SESSION["csrf_tokens"][$token] = false;
34+
return $token;
35+
}
36+
37+
public static function validate(string $token): bool
38+
{
39+
self::ensureSessionCSRFTokensSanity();
40+
if ($token === "") {
41+
UnityHTTPD::errorLog("empty CSRF token", "");
42+
return false;
43+
}
44+
if (!array_key_exists($token, $_SESSION["csrf_tokens"])) {
45+
UnityHTTPD::errorLog("unknown CSRF token", $token);
46+
return false;
47+
}
48+
$entry = $_SESSION["csrf_tokens"][$token];
49+
if ($entry === true) {
50+
UnityHTTPD::errorLog("reused CSRF token", $token);
51+
return false;
52+
}
53+
$_SESSION["csrf_tokens"][$token] = true;
54+
return true;
55+
}
56+
57+
public static function clear(): void
58+
{
59+
if (!isset($_SESSION)) {
60+
return;
61+
}
62+
if (array_key_exists("csrf_tokens", $_SESSION)) {
63+
unset($_SESSION["csrf_tokens"]);
64+
}
65+
$_SESSION["csrf_tokens"] = [];
66+
}
67+
}

‎resources/lib/UnityHTTPD.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,4 +390,18 @@ public static function deleteMessage(UnityHTTPDMessageLevel $level, string $titl
390390
unset($_SESSION["messages"][$index]);
391391
$_SESSION["messages"] = array_values($_SESSION["messages"]);
392392
}
393+
394+
public static function validatePostCSRFToken(): void
395+
{
396+
$token = self::getPostData("csrf_token");
397+
if (!CSRFToken::validate($token)) {
398+
self::badRequest("CSRF token validation failed", data: ["token" => $token]);
399+
}
400+
}
401+
402+
public static function getCSRFTokenHiddenFormInput(): string
403+
{
404+
$token = htmlspecialchars(CSRFToken::generate());
405+
return "<input type='hidden' name='csrf_token' value='$token'>";
406+
}
393407
}

‎resources/templates/header.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
use UnityWebPortal\lib\UnityHTTPD;
44

55
if ($_SERVER["REQUEST_METHOD"] == "POST") {
6+
// another page should have already validated and we can't validate the same token twice
7+
// UnityHTTPD::validatePostCSRFToken();
68
if (
79
($_SESSION["is_admin"] ?? false) == true
810
&& ($_POST["form_type"] ?? null) == "clearView"
@@ -179,10 +181,12 @@
179181
&& isset($_SESSION["viewUser"])
180182
) {
181183
$viewUser = $_SESSION["viewUser"];
184+
$CSRFTokenHiddenFormInput = UnityHTTPD::getCSRFTokenHiddenFormInput();
182185
echo "
183186
<div id='viewAsBar'>
184187
<span>You are accessing the web portal as the user <strong>$viewUser</strong></span>
185188
<form method='POST' action=''>
189+
$CSRFTokenHiddenFormInput
186190
<input type='hidden' name='form_type' value='clearView'>
187191
<input type='hidden' name='uid' value='$viewUser'>
188192
<input type='submit' value='Return to My User'>

‎test/functional/ViewAsUserTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public function _testViewAsUser(array $beforeUser, array $afterUser)
2525
// now we should be new user
2626
$this->assertEquals($afterUid, $USER->uid);
2727
// $this->assertTrue($_SESSION["user_exists"]);
28-
http_post(__DIR__ . "/../../resources/templates/header.php", [
28+
http_post(__DIR__ . "/../../webroot/panel/account.php", [
2929
"form_type" => "clearView",
3030
]);
3131
$this->assertArrayNotHasKey("viewUser", $_SESSION);

‎test/phpunit-bootstrap.php‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
require_once __DIR__ . "/../resources/lib/UnityWebhook.php";
1616
require_once __DIR__ . "/../resources/lib/UnityGithub.php";
1717
require_once __DIR__ . "/../resources/lib/utils.php";
18+
require_once __DIR__ . "/../resources/lib/CSRFToken.php";
1819
require_once __DIR__ . "/../resources/lib/exceptions/NoDieException.php";
1920
require_once __DIR__ . "/../resources/lib/exceptions/SSOException.php";
2021
require_once __DIR__ . "/../resources/lib/exceptions/ArrayKeyException.php";
@@ -25,6 +26,7 @@
2526
require_once __DIR__ . "/../resources/lib/exceptions/EncodingConversionException.php";
2627
require_once __DIR__ . "/../resources/lib/exceptions/UnityHTTPDMessageNotFoundException.php";
2728

29+
use UnityWebPortal\lib\CSRFToken;
2830
use UnityWebPortal\lib\UnityGroup;
2931
use UnityWebPortal\lib\UnityHTTPD;
3032
use UnityWebPortal\lib\UnitySQL;
@@ -97,7 +99,7 @@ function switchUser(
9799
ensure(!is_null($USER));
98100
}
99101

100-
function http_post(string $phpfile, array $post_data, bool $enforce_PRG = true): void
102+
function http_post(string $phpfile, array $post_data, bool $enforce_PRG = true, bool $do_generate_csrf_token = true): void
101103
{
102104
global $LDAP,
103105
$SQL,
@@ -115,6 +117,9 @@ function http_post(string $phpfile, array $post_data, bool $enforce_PRG = true):
115117
$_SERVER["REQUEST_METHOD"] = "POST";
116118
$_SERVER["PHP_SELF"] = preg_replace("/.*webroot\//", "/", $phpfile);
117119
$_SERVER["REQUEST_URI"] = preg_replace("/.*webroot\//", "/", $phpfile); // Slightly imprecise because it doesn't include get parameters
120+
if (!array_key_exists("csrf_token", $post_data) && $do_generate_csrf_token) {
121+
$post_data["csrf_token"] = CSRFToken::generate();
122+
}
118123
$_POST = $post_data;
119124
ob_start();
120125
$post_did_redirect_or_die = false;

‎test/unit/CSRFTokenTest.php‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
<?php
2+
use PHPUnit\Framework\TestCase;
3+
use UnityWebPortal\lib\CSRFToken;
4+
5+
class CSRFTokenTest extends TestCase
6+
{
7+
protected function setUp(): void
8+
{
9+
session_id(uniqid());
10+
session_start();
11+
$_SESSION["csrf_tokens"] = [];
12+
}
13+
14+
protected function tearDown(): void
15+
{
16+
CSRFToken::clear();
17+
session_write_close();
18+
session_id(uniqid());
19+
}
20+
21+
public function testGenerateCreatesToken(): void
22+
{
23+
$token = CSRFToken::generate();
24+
$this->assertIsString($token);
25+
$this->assertEquals(64, strlen($token));
26+
$this->assertMatchesRegularExpression('/^[0-9a-f]{64}$/', $token);
27+
}
28+
29+
public function testGenerateStoresTokenInSession(): void
30+
{
31+
$token = CSRFToken::generate();
32+
$this->assertArrayHasKey("csrf_tokens", $_SESSION);
33+
$this->assertArrayHasKey($token, $_SESSION["csrf_tokens"]);
34+
$this->assertFalse($_SESSION["csrf_tokens"][$token]);
35+
}
36+
37+
public function testValidateWithValidToken(): void
38+
{
39+
$token = CSRFToken::generate();
40+
$this->assertTrue(CSRFToken::validate($token));
41+
$this->assertTrue($_SESSION["csrf_tokens"][$token]);
42+
}
43+
44+
public function testValidateWithInvalidToken(): void
45+
{
46+
CSRFToken::generate();
47+
$this->assertFalse(CSRFToken::validate("invalid_token"));
48+
}
49+
50+
public function testValidateWithEmptyToken(): void
51+
{
52+
CSRFToken::generate();
53+
$this->assertFalse(CSRFToken::validate(""));
54+
}
55+
56+
public function testValidateWithoutSessionToken(): void
57+
{
58+
$this->assertFalse(CSRFToken::validate("any_token"));
59+
}
60+
61+
public function testClearRemovesToken(): void
62+
{
63+
CSRFToken::generate();
64+
$this->assertNotEmpty($_SESSION["csrf_tokens"]);
65+
CSRFToken::clear();
66+
$this->assertEmpty($_SESSION["csrf_tokens"]);
67+
}
68+
69+
public function testMultipleTokenGenerations(): void
70+
{
71+
$token1 = CSRFToken::generate();
72+
$token2 = CSRFToken::generate();
73+
$this->assertNotEquals($token1, $token2);
74+
}
75+
76+
public function testTokenIsSingleUse(): void
77+
{
78+
$token = CSRFToken::generate();
79+
$this->assertTrue(CSRFToken::validate($token));
80+
$this->assertFalse(CSRFToken::validate($token));
81+
$this->assertTrue($_SESSION["csrf_tokens"][$token]);
82+
}
83+
}

‎webroot/admin/ajax/get_group_members.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
echo "<td>$uid</td>";
3232
echo "<td><a href='mailto:$mail'>$mail</a></td>";
3333
echo "<td>";
34+
$CSRFTokenHiddenFormInput = UnityHTTPD::getCSRFTokenHiddenFormInput();
3435
echo "
3536
<form
3637
action=''
@@ -39,6 +40,7 @@
3940
return confirm(\"Are you sure you want to remove $uid from this group?\");
4041
'
4142
>
43+
$CSRFTokenHiddenFormInput
4244
<input type='hidden' name='form_type' value='remUserChild'>
4345
<input type='hidden' name='uid' value='$uid'>
4446
<input type='hidden' name='pi' value='$group->gid'>
@@ -62,9 +64,11 @@
6264
echo "<td>$user->uid</td>";
6365
echo "<td><a href='mailto:$email'>$email</a></td>";
6466
echo "<td>";
67+
$CSRFTokenHiddenFormInput = UnityHTTPD::getCSRFTokenHiddenFormInput();
6568
echo
6669
"<form action='' method='POST'
6770
onsubmit='return confirm(\"Are you sure you want to approve $user->uid ?\");'>
71+
$CSRFTokenHiddenFormInput
6872
<input type='hidden' name='form_type' value='reqChild'>
6973
<input type='hidden' name='uid' value='$user->uid'>
7074
<input type='hidden' name='pi' value='$group->gid'>

0 commit comments

Comments
 (0)