Skip to content

Commit 7adb9d9

Browse files
simonLeary42simonLeary42
authored
CSRF protection (#389)
1 parent 75fb513 commit 7adb9d9

File tree

20 files changed

+248
-16
lines changed

20 files changed

+248
-16
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
@@ -25,6 +25,7 @@
2525
require_once __DIR__ . "/lib/UnityWebhook.php";
2626
require_once __DIR__ . "/lib/UnityGithub.php";
2727
require_once __DIR__ . "/lib/utils.php";
28+
require_once __DIR__ . "/lib/CSRFToken.php";
2829
require_once __DIR__ . "/lib/exceptions/NoDieException.php";
2930
require_once __DIR__ . "/lib/exceptions/SSOException.php";
3031
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: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
require_once __DIR__ . "/../resources/lib/UnityWebhook.php";
1717
require_once __DIR__ . "/../resources/lib/UnityGithub.php";
1818
require_once __DIR__ . "/../resources/lib/utils.php";
19+
require_once __DIR__ . "/../resources/lib/CSRFToken.php";
1920
require_once __DIR__ . "/../resources/lib/exceptions/NoDieException.php";
2021
require_once __DIR__ . "/../resources/lib/exceptions/SSOException.php";
2122
require_once __DIR__ . "/../resources/lib/exceptions/ArrayKeyException.php";
@@ -26,6 +27,7 @@
2627
require_once __DIR__ . "/../resources/lib/exceptions/EncodingConversionException.php";
2728
require_once __DIR__ . "/../resources/lib/exceptions/UnityHTTPDMessageNotFoundException.php";
2829

30+
use UnityWebPortal\lib\CSRFToken;
2931
use UnityWebPortal\lib\UnityGroup;
3032
use UnityWebPortal\lib\UnityHTTPD;
3133
use UnityWebPortal\lib\UnitySQL;
@@ -98,8 +100,12 @@ function switchUser(
98100
ensure(!is_null($USER));
99101
}
100102

101-
function http_post(string $phpfile, array $post_data, bool $enforce_PRG = true): void
102-
{
103+
function http_post(
104+
string $phpfile,
105+
array $post_data,
106+
bool $enforce_PRG = true,
107+
bool $do_generate_csrf_token = true,
108+
): void {
103109
global $LDAP,
104110
$SQL,
105111
$MAILER,
@@ -116,6 +122,9 @@ function http_post(string $phpfile, array $post_data, bool $enforce_PRG = true):
116122
$_SERVER["REQUEST_METHOD"] = "POST";
117123
$_SERVER["PHP_SELF"] = preg_replace("/.*webroot\//", "/", $phpfile);
118124
$_SERVER["REQUEST_URI"] = preg_replace("/.*webroot\//", "/", $phpfile); // Slightly imprecise because it doesn't include get parameters
125+
if (!array_key_exists("csrf_token", $post_data) && $do_generate_csrf_token) {
126+
$post_data["csrf_token"] = CSRFToken::generate();
127+
}
119128
$_POST = $post_data;
120129
ob_start();
121130
$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)