Просмотр исходного кода

Use timing safe comparison for tokens everywhere (#8945)

I already changed some places that were using normal string comparison to hash_equals before, but didn't check other places.

Now the checks should be consistent.
Inverle 2 дней назад
Родитель
Сommit
931d62ab08
5 измененных файлов с 6 добавлено и 6 удалено
  1. 1 1
      app/Controllers/userController.php
  2. 2 2
      app/Models/Auth.php
  3. 1 1
      p/api/fever.php
  4. 1 1
      p/api/greader.php
  5. 1 1
      p/api/query.php

+ 1 - 1
app/Controllers/userController.php

@@ -605,7 +605,7 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController {
 		}
 
 		if ($token != '') {
-			if ($user_config->email_validation_token !== $token) {
+			if (!hash_equals($user_config->email_validation_token, $token)) {
 				Minz_Request::bad(
 					_t('user.email.validation.feedback.wrong_token'),
 					['c' => 'user', 'a' => 'validateEmail']

+ 2 - 2
app/Models/Auth.php

@@ -216,9 +216,9 @@ class FreshRSS_Auth {
 	public static function isCsrfOk(?string $token = null): bool {
 		$csrf = Minz_Session::paramString('csrf');
 		if ($token === null) {
-			$token = $_POST['_csrf'] ?? '';
+			$token = is_string($_POST['_csrf'] ?? null) ? $_POST['_csrf'] : '';
 		}
-		return $token != '' && $token === $csrf;
+		return $token != '' && hash_equals($csrf, $token);
 	}
 
 	public static function needsReauth(): bool {

+ 1 - 1
p/api/fever.php

@@ -176,7 +176,7 @@ final class FeverAPI
 			if ($username != false) {
 				$username = trim($username);
 				FreshRSS_Context::initUser($username);
-				if ($feverKey === FreshRSS_Context::userConf()->feverKey && FreshRSS_Context::userConf()->enabled) {
+				if (hash_equals(FreshRSS_Context::userConf()->feverKey, $feverKey) && FreshRSS_Context::userConf()->enabled) {
 					Minz_Translate::init(FreshRSS_Context::userConf()->language);
 					$this->entryDAO = FreshRSS_Factory::createEntryDao();
 					$this->feedDAO = FreshRSS_Factory::createFeedDao();

+ 1 - 1
p/api/greader.php

@@ -255,7 +255,7 @@ final class GReaderAPI {
 			$token === 'x')) { //Reeder
 			return true;
 		}
-		if ($token === str_pad(sha1(FreshRSS_Context::systemConf()->salt . $user . $conf->apiPasswordHash), 57, 'Z')) {
+		if (hash_equals(str_pad(sha1(FreshRSS_Context::systemConf()->salt . $user . $conf->apiPasswordHash), 57, 'Z'), $token)) {
 			return true;
 		}
 		Minz_Log::warning('Invalid POST token: ' . $token, API_LOG);

+ 1 - 1
p/api/query.php

@@ -66,7 +66,7 @@ Minz_ExtensionManager::enableByList(FreshRSS_Context::userConf()->extensions_ena
 $query = null;
 $userSearch = null;
 foreach (FreshRSS_Context::userConf()->queries as $raw_query) {
-	if (!empty($raw_query['token']) && $raw_query['token'] === $token) {
+	if (!empty($raw_query['token']) && hash_equals($raw_query['token'], $token)) {
 		switch ($format) {
 			case 'atom':
 			case 'greader':