From: Adolf Belka <adolf.belka@ipfire.org>
Date: Sat, 10 May 2025 10:30:56 +0000 (+0200)
Subject: chpasswd.cgi: Fixes bug12755 - v3 with password verification correction
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5e11bfd87725698270de1343ae664b0b872d0d12;p=ipfire-2.x.git

chpasswd.cgi: Fixes bug12755 - v3 with password verification correction

- v3 version based on feedback from @Michael to use the status value returned from
   using the htpasswd command.
- Also simplified the whole section to carry out the change if the status is 0, ie all
   went well, otherwise give an error but without identifying if the error is in the
   username or the password. This makes it more secure as any attacker only knows it
   failed and doesn't know if any part of the authentication was correct or not.
- Changed the error messages in line with this so the language file changes are in the
   other part of this patch set submission.
- Tested out on my vm test bed and worked fine. If the username was incorrect or the
   password was incorrect or both were incorrect the same error message is given. If
   both are correct then the update is carried out.

Fixes: bug12755
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
---

diff --git a/html/cgi-bin/chpasswd.cgi b/html/cgi-bin/chpasswd.cgi
index c00caca20..0a1a5c9e3 100644
--- a/html/cgi-bin/chpasswd.cgi
+++ b/html/cgi-bin/chpasswd.cgi
@@ -74,19 +74,14 @@ if ($cgiparams{'SUBMIT'} eq $tr{'advproxy chgwebpwd change password'})
 		goto ERROR;
 	}
 
-       # Check if a user with this name and password exists in the userdb file
-       # and if it does then change the password to the new one
-       my $user = &General::system_output("grep", "$cgiparams{'USERNAME'}", "$userdb");
-       my $old_password = &General::system_output("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}");
-       if (!$user) {
-               $errormessage = $tr{'advproxy errmsg invalid user'};
-               goto ERROR;
-       } elsif (!$old_password) {
-                $errormessage = $tr{'advproxy errmsg password incorrect'};
-                goto ERROR;
-       } else {
-               &General::system("/usr/bin/htpasswd", "-bB", "-C 10", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'NEW_PASSWORD_1'}");
-       }
+	# If the htpasswd verification status is 0 then update the database
+	# otherwise respond with an error message.
+	if (&General::system("/usr/bin/htpasswd", "-bv", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'OLD_PASSWORD'}") != 0) {
+		$errormessage = $tr{'advproxy errmsg invalid user/password'};
+		goto ERROR;
+	} else {
+		&General::system("/usr/bin/htpasswd", "-bB", "-C 10", "$userdb", "$cgiparams{'USERNAME'}", "$cgiparams{'NEW_PASSWORD_1'}");
+	}
 
 	$success = 1;
 	undef %cgiparams;