Skip to content

Commit 940b55c

Browse files
committed
[SECURITY] Added CSRF protection to form
1 parent d08f18c commit 940b55c

4 files changed

Lines changed: 23 additions & 10 deletions

File tree

Classes/Controller/PasswordController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public function editAction()
5656
public function initializeUpdateAction()
5757
{
5858
$changePasswordArray = $this->request->getArgument('changePassword');
59-
$changeHmac = $changePasswordArray['changeHmac'] ? $changePasswordArray['changeHmac'] : '';
59+
$changeHmac = $changePasswordArray['changeHmac'] ? (string)$changePasswordArray['changeHmac'] : '';
6060
if (!$this->frontendUserService->validateChangeHmac($changeHmac)) {
6161
throw new \TYPO3\CMS\Extbase\Security\Exception\InvalidHashException(
6262
'Possible CSRF detected. Ensure a valid "changeHmac" is provided.',

Classes/Domain/Model/Dto/ChangePassword.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public function setCurrentPassword(string $currentPassword)
8585
/**
8686
* @return string
8787
*/
88-
public function getChangeHmac()
88+
public function getChangeHmac(): string
8989
{
9090
return $this->changeHmac;
9191
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?php
2+
declare(strict_types=1);
3+
namespace Derhansen\FeChangePwd\Service;
4+
5+
use Exception;
6+
7+
/**
8+
* Invalid user exception
9+
*/
10+
class InvalidUserException extends Exception
11+
{
12+
}

Classes/Service/FrontendUserService.php

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,19 @@ public function updatePassword(string $newPassword)
116116
*
117117
* @return string
118118
*/
119-
public function getChangeHmac()
119+
public function getChangeHmac(): string
120120
{
121121
if (!$this->isUserLoggedIn()) {
122122
return '';
123123
}
124124

125125
$userUid = $this->getFrontendUser()->user['uid'];
126-
return GeneralUtility::hmac('fe_user_' . $userUid, 'fe_change_pwd');
126+
if (!is_int($userUid) || (int)$userUid <= 0) {
127+
throw new InvalidUserException('The fe_user uid is not a positive number.', 1574102778917);
128+
}
129+
130+
$tstamp = $this->getFrontendUser()->user['tstamp'];
131+
return GeneralUtility::hmac('fe_user_' . $userUid . '_' . $tstamp, 'fe_change_pwd');
127132
}
128133

129134
/**
@@ -132,13 +137,9 @@ public function getChangeHmac()
132137
* @param string $changeHmac
133138
* @return bool
134139
*/
135-
public function validateChangeHmac($changeHmac)
140+
public function validateChangeHmac(string $changeHmac): bool
136141
{
137-
if ($changeHmac !== '' && $changeHmac === $this->getChangeHmac()) {
138-
return true;
139-
} else {
140-
return false;
141-
}
142+
return is_string($changeHmac) && $changeHmac !== '' && hash_equals($this->getChangeHmac(), $changeHmac);
142143
}
143144

144145
/**

0 commit comments

Comments
 (0)