CVE-2023-3173: Improper Restriction of Excessive Authentication Attempts (CWE-307)
value: Improper Restriction of Excessive Authentication Attempts in GitHub repository froxlor/froxlor prior to 2.0.20.
CWE description: The product does not implement sufficient measures to prevent multiple failed authentication attempts within a short time frame, making it more susceptible to brute force attacks.
CVSS v3 base score: 9.8
Published: 2023-06-09T02:15Z
Repository: https://github.com/froxlor/froxlor
Language: PHP
Files changed: index.php, lng/de.lng.php, lng/en.lng.php, templates/Froxlor/login/enter2fa.html.twig
Commit date: 2023-05-12 10:36:27 +0200
Commit message: add same loginfail restrictions for entering 2fa code as for user/pwd login

Signed-off-by: Michael Kaufmann <d00p@froxlor.org>

Diff: diff --git a/index.php b/index.php
index a701bfaa04..d59146e361 100644
--- a/index.php
+++ b/index.php
@@ -51,13 +51,19 @@ if ($action == '2fa_entercode') {
 	if (!isset($_SESSION) || !isset($_SESSION['secret_2fa'])) {
 		// no session - redirect to index
 		Response::redirectTo('index.php');
 		exit();
 	}
+	$smessage = isset($_GET['showmessage']) ? (int)$_GET['showmessage'] : 0;
+	$message = "";
+	if ($smessage > 0) {
+		$message = lng('error.2fa_wrongcode');
+	}
 	// show template to enter code
 	UI::view('login/enter2fa.html.twig', [
-		'pagetitle' => lng('login.2fa')
+		'pagetitle' => lng('login.2fa'),
+		'message' => $message
 	]);
 } elseif ($action == '2fa_verify') {
 	// verify code from 2fa code-enter form
 	if (!isset($_SESSION) || !isset($_SESSION['secret_2fa'])) {
 		// no session - redirect to index
@@ -66,29 +72,29 @@ if ($action == '2fa_entercode') {
 	}
 	$code = isset($_POST['2fa_code']) ? $_POST['2fa_code'] : null;
 	// verify entered code
 	$tfa = new FroxlorTwoFactorAuth('Froxlor ' . Settings::Get('system.hostname'));
 	$result = ($_SESSION['secret_2fa'] == 'email' ? true : $tfa->verifyCode($_SESSION['secret_2fa'], $code, 3));
+	// get user-data
+	$table = $_SESSION['uidtable_2fa'];
+	$field = $_SESSION['uidfield_2fa'];
+	$uid = $_SESSION['uid_2fa'];
+	$isadmin = $_SESSION['unfo_2fa'];
 	// either the code is valid when using authenticator-app, or we will select userdata by id and entered code
 	// which is temporarily stored for the customer when using email-2fa
 	if ($result) {
-		// get user-data
-		$table = $_SESSION['uidtable_2fa'];
-		$field = $_SESSION['uidfield_2fa'];
-		$uid = $_SESSION['uid_2fa'];
-		$isadmin = $_SESSION['unfo_2fa'];
 		$sel_param = [
 			'uid' => $uid
 		];
 		if ($_SESSION['secret_2fa'] == 'email') {
 			// verify code by selecting user by id and the temp. stored code,
 			// so only if it's the correct code, we get the user-data
-			$sel_stmt = Database::prepare("SELECT * FROM $table WHERE `" . $field . "` = :uid AND `data_2fa` = :code");
+			$sel_stmt = Database::prepare("SELECT * FROM " . $table . " WHERE `" . $field . "` = :uid AND `data_2fa` = :code");
 			$sel_param['code'] = $code;
 		} else {
 			// Authenticator-verification has already happened at this point, so just get the user-data
-			$sel_stmt = Database::prepare("SELECT * FROM $table WHERE `" . $field . "` = :uid");
+			$sel_stmt = Database::prepare("SELECT * FROM " . $table . " WHERE `" . $field . "` = :uid");
 		}
 		$userinfo = Database::pexecute_first($sel_stmt, $sel_param);
 		// whoops, no (valid) user? Start again
 		if (empty($userinfo)) {
 			Response::redirectTo('index.php', [
@@ -106,19 +112,54 @@ if ($action == '2fa_entercode') {
 			]);
 		}
 
 		// when using email-2fa, remove the one-time-code
 		if ($userinfo['type_2fa'] == '1') {
-			$del_stmt = Database::prepare("UPDATE $table SET `data_2fa` = '' WHERE `" . $field . "` = :uid");
+			$del_stmt = Database::prepare("UPDATE " . $table . " SET `data_2fa` = '' WHERE `" . $field . "` = :uid");
 			$userinfo = Database::pexecute_first($del_stmt, [
 				'uid' => $uid
 			]);
 		}
 		exit();
 	}
+	// wrong 2fa code - treat like "wrong password"
+	$stmt = Database::prepare("
+		UPDATE " . $table . "
+		SET `lastlogin_fail`= :lastlogin_fail, `loginfail_count`=`loginfail_count`+1
+		WHERE `" . $field . "`= :uid
+	");
+	Database::pexecute($stmt, [
+		"lastlogin_fail" => time(),
+		"uid" => $uid
+	]);
+
+	// get data for processing further
+	$stmt = Database::prepare("
+		SELECT `loginname`, `loginfail_count`, `lastlogin_fail` FROM " . $table . "
+		WHERE `" . $field . "`= :uid
+	");
+	$fail_user = Database::pexecute_first($stmt, [
+		"uid" => $uid
+	]);
+
+	if ($fail_user['loginfail_count'] >= Settings::Get('login.maxloginattempts') && $fail_user['lastlogin_fail'] > (time() - Settings::Get('login.deactivatetime'))) {
+		// Log failed login
+		$rstlog = FroxlorLogger::getInstanceOf([
+			'loginname' => $_SERVER['REMOTE_ADDR']
+		]);
+		$rstlog->logAction(FroxlorLogger::LOGIN_ACTION, LOG_WARNING, "User '" . $fail_user['loginname'] . "' entered wrong 2fa code too often.");
+		unset($fail_user);
+		Response::redirectTo('index.php', [
+			'showmessage' => '3'
+		]);
+		exit();
+	}
+	unset($fail_user);
+	// back to form
 	Response::redirectTo('index.php', [
-		'showmessage' => '2'
+		'action' => '2fa_entercode',
+		'showmessage' => '1'
 	]);
 	exit();
 } elseif ($action == 'login') {
 	if (isset($_POST['send']) && $_POST['send'] == 'send') {
 		$loginname = Validate::validate($_POST['loginname'], 'loginname');
diff --git a/lng/de.lng.php b/lng/de.lng.php
index 83aa897f05..c331ccd5c5 100644
--- a/lng/de.lng.php
+++ b/lng/de.lng.php
@@ -924,10 +924,11 @@ return [
 		'pathmustberelative' => 'Der Benutzer hat nicht die benötigten Berechtigungen, um Pfade außerhalb des Kunden-Heimatverzeichnisses anzugeben. Bitte einen relativen Pfad angeben (kein führendes /).',
 		'mysqlserverstillhasdbs' => 'Datenbank-Server kann für den Kunden nicht entfernt werden, da sich dort noch Datenbanken befinden.',
 		'domaincannotbeedited' => 'Keine Berechtigung, um die Domain %s zu bearbeiten',
 		'invalidcronjobintervalvalue' => 'Cronjob Intervall muss einer der folgenden Werte sein: %s',
 		'phpgdextensionnotavailable' => 'Die PHP GD Extension ist nicht verfügbar. Bild-Daten können nicht validiert werden.',
+		'2fa_wrongcode' => 'Der angegebene Code ist nicht korrekt',
 	],
 	'extras' => [
 		'description' => 'Hier können Sie zusätzliche Extras einrichten, wie zum Beispiel einen Verzeichnisschutz.<br />Die Änderungen sind erst nach einer kurzen Zeit wirksam.',
 		'directoryprotection_add' => 'Verzeichnisschutz anlegen',
 		'view_directory' => 'Verzeichnis anzeigen',
diff --git a/lng/en.lng.php b/lng/en.lng.php
index e01d50e688..e4645ca372 100644
--- a/lng/en.lng.php
+++ b/lng/en.lng.php
@@ -993,10 +993,11 @@ return [
 		'pathmustberelative' => 'The user does not have the permission to specify directories outside the customers home-directory. Please specify a relative path (no leading /).',
 		'mysqlserverstillhasdbs' => 'Cannot remove database server from customers allow-list as there are still databases on it.',
 		'domaincannotbeedited' => 'You are not permitted to edit the domain %s',
 		'invalidcronjobintervalvalue' => 'Cronjob interval must be one of: %s',
 		'phpgdextensionnotavailable' => 'The PHP GD extension is not available. Unable to validate image-data',
+		'2fa_wrongcode' => 'The code entered is not valid',
 	],
 	'extras' => [
 		'description' => 'Here you can add some extras, for example directory protection.<br />The system will need some time to apply the new settings after every change.',
 		'directoryprotection_add' => 'Add directory protection',
 		'view_directory' => 'Display directory content',
diff --git a/templates/Froxlor/login/enter2fa.html.twig b/templates/Froxlor/login/enter2fa.html.twig
index 75dee1cc32..a185c9868a 100644
--- a/templates/Froxlor/login/enter2fa.html.twig
+++ b/templates/Froxlor/login/enter2fa.html.twig
@@ -8,10 +8,17 @@
 
 				<div class="card shadow">
 					<div class="card-body">
 						<h5 class="card-title">{{ pagetitle }}</h5>
 
+						{% if message is not empty %}
+							<div class="alert alert-danger" role="alert">
+								<h4 class="alert-heading">{{ lng('error.error') }}</h4>
+								<p>{{ message|raw }}</p>
+							</div>
+						{% endif %}
+
 						<div class="mb-3">
 							<label for="2fa_code" class="col-form-label">{{ lng('login.2facode') }}</label>
 							<input class="form-control" type="text" name="2fa_code" id="2fa_code" value="" autocomplete="off" autofocus required/>
 						</div>
 

Vulnerable code: 		'pagetitle' => lng('login.2fa')
		// get user-data
		$table = $_SESSION['uidtable_2fa'];
		$field = $_SESSION['uidfield_2fa'];
		$uid = $_SESSION['uid_2fa'];
		$isadmin = $_SESSION['unfo_2fa'];
			$sel_stmt = Database::prepare("SELECT * FROM $table WHERE `" . $field . "` = :uid AND `data_2fa` = :code");
			$sel_stmt = Database::prepare("SELECT * FROM $table WHERE `" . $field . "` = :uid");
			$del_stmt = Database::prepare("UPDATE $table SET `data_2fa` = '' WHERE `" . $field . "` = :uid");
		'showmessage' => '2'
Fixed code: 	$smessage = isset($_GET['showmessage']) ? (int)$_GET['showmessage'] : 0;
	$message = "";
	if ($smessage > 0) {
		$message = lng('error.2fa_wrongcode');
	}
		'pagetitle' => lng('login.2fa'),
		'message' => $message
	// get user-data
	$table = $_SESSION['uidtable_2fa'];
	$field = $_SESSION['uidfield_2fa'];
	$uid = $_SESSION['uid_2fa'];
	$isadmin = $_SESSION['unfo_2fa'];
			$sel_stmt = Database::prepare("SELECT * FROM " . $table . " WHERE `" . $field . "` = :uid AND `data_2fa` = :code");
			$sel_stmt = Database::prepare("SELECT * FROM " . $table . " WHERE `" . $field . "` = :uid");
			$del_stmt = Database::prepare("UPDATE " . $table . " SET `data_2fa` = '' WHERE `" . $field . "` = :uid");
	// wrong 2fa code - treat like "wrong password"
	$stmt = Database::prepare("
		UPDATE " . $table . "
		SET `lastlogin_fail`= :lastlogin_fail, `loginfail_count`=`loginfail_count`+1
		WHERE `" . $field . "`= :uid
	");
	Database::pexecute($stmt, [
		"lastlogin_fail" => time(),
		"uid" => $uid
	]);

	// get data for processing further
	$stmt = Database::prepare("
		SELECT `loginname`, `loginfail_count`, `lastlogin_fail` FROM " . $table . "
		WHERE `" . $field . "`= :uid
	");
	$fail_user = Database::pexecute_first($stmt, [
		"uid" => $uid
	]);

	if ($fail_user['loginfail_count'] >= Settings::Get('login.maxloginattempts') && $fail_user['lastlogin_fail'] > (time() - Settings::Get('login.deactivatetime'))) {
		// Log failed login
		$rstlog = FroxlorLogger::getInstanceOf([
			'loginname' => $_SERVER['REMOTE_ADDR']
		]);
		$rstlog->logAction(FroxlorLogger::LOGIN_ACTION, LOG_WARNING, "User '" . $fail_user['loginname'] . "' entered wrong 2fa code too often.");
		unset($fail_user);
		Response::redirectTo('index.php', [
			'showmessage' => '3'
		]);
		exit();
	}
	unset($fail_user);
	// back to form
		'action' => '2fa_entercode',
		'showmessage' => '1'
		'2fa_wrongcode' => 'Der angegebene Code ist nicht korrekt',
		'2fa_wrongcode' => 'The code entered is not valid',
						{% if message is not empty %}
							<div class="alert alert-danger" role="alert">
								<h4 class="alert-heading">{{ lng('error.error') }}</h4>
								<p>{{ message|raw }}</p>
							</div>
						{% endif %}