CVE-2022-3376: Weak Password Requirements (CWE-521)
value: Weak Password Requirements in GitHub repository ikus060/rdiffweb prior to 2.5.0a4.
CWE description: The product does not require that users should have strong passwords, which makes it easier for attackers to compromise user accounts.
CVSS v3 base score: 5.3
Published: 2022-10-06T18:16Z
Repository: https://github.com/ikus060/rdiffweb
Language: Python
Files changed: README.md, rdiffweb/controller/page_pref_general.py, rdiffweb/controller/tests/test_page_prefs_general.py
Commit date: 2022-10-04 08:02:18 -0400
Commit message: Enforce password policy new password cannot be set as new password

Diff: diff --git a/README.md b/README.md
index e6531be45..09c51a982 100644
--- a/README.md
+++ b/README.md
@@ -131,10 +131,11 @@ This next release focus on two-factor-authentication as a measure to increase se
 * Add two-factor authentication with email verification #201
 * Generate a new session on login and 2FA #220
 * Enforce permission on /etc/rdiffweb configuration folder
 * Enforce validation on fullname, username and email
 * Limit incorrect attempts to change the user's password to prevent brute force attacks #225 [CVE-2022-3273](https://nvd.nist.gov/vuln/detail/CVE-2022-3273)
+* Enforce password policy new password cannot be set as new password [CVE-2022-3376](https://nvd.nist.gov/vuln/detail/CVE-2022-3376)
 
 Breaking changes:
 
 * Drop Ubuntu Hirsute & Impish (End-of-life)
 * `session-dir` is deprecated and should be replace by `rate-limit-dir`. User's session are stored in database.
diff --git a/rdiffweb/controller/page_pref_general.py b/rdiffweb/controller/page_pref_general.py
index 453378ba4..9ae459e2f 100644
--- a/rdiffweb/controller/page_pref_general.py
+++ b/rdiffweb/controller/page_pref_general.py
@@ -86,10 +86,17 @@ class UserPasswordForm(CherryForm):
 
     def is_submitted(self):
         # Validate only if action is set_profile_info
         return super().is_submitted() and self.action.data == 'set_password'
 
+    def validate_new(self, field):
+        """
+        Make sure new password if not equals to old password.
+        """
+        if self.new.data and self.new.data == self.current.data:
+            raise ValueError(_('The new password must be different from the current password.'))
+
     def populate_obj(self, user):
         # Check if current password is "valid" if Not, rate limit the
         # number of attempts and logout user after too many invalid attempts.
         if not user.validate_password(self.current.data):
             cherrypy.session[CHANGE_PASSWORD_ATTEMPTS] = cherrypy.session.get(CHANGE_PASSWORD_ATTEMPTS, 0) + 1
@@ -100,21 +107,23 @@ class UserPasswordForm(CherryForm):
                 flash(
                     _("You were logged out because you entered the wrong password too many times."),
                     level='warning',
                 )
                 raise cherrypy.HTTPRedirect('/login/')
-            flash(_("Wrong current password."), level='warning')
-        else:
-            # Clear number of attempts
-            if CHANGE_PASSWORD_ATTEMPTS in cherrypy.session:
-                del cherrypy.session[CHANGE_PASSWORD_ATTEMPTS]
-            # If Valid, update password
-            try:
-                user.set_password(self.new.data)
-                flash(_("Password updated successfully."), level='success')
-            except ValueError as e:
-                flash(str(e), level='warning')
+            self.current.errors = [_("Wrong current password.")]
+            return False
+
+        # Clear number of attempts
+        if CHANGE_PASSWORD_ATTEMPTS in cherrypy.session:
+            del cherrypy.session[CHANGE_PASSWORD_ATTEMPTS]
+
+        try:
+            user.set_password(self.new.data)
+            return True
+        except ValueError as e:
+            self.new.errors = [str(e)]
+            return False
 
 
 class RefreshForm(CherryForm):
     action = HiddenField(default='update_repos')
     update_repos = SubmitField(
@@ -149,15 +158,18 @@ class PagePrefsGeneral(Controller):
         refresh_form = RefreshForm()
         if profile_form.is_submitted():
             if profile_form.validate():
                 profile_form.populate_obj(self.app.currentuser)
                 flash(_("Profile updated successfully."), level='success')
+                raise cherrypy.HTTPRedirect("")
             else:
                 flash(profile_form.error_message, level='error')
         elif password_form.is_submitted():
             if password_form.validate():
-                password_form.populate_obj(self.app.currentuser)
+                if password_form.populate_obj(self.app.currentuser):
+                    flash(_("Password updated successfully."), level='success')
+                    raise cherrypy.HTTPRedirect("")
             else:
                 flash(password_form.error_message, level='error')
         elif refresh_form.is_submitted():
             if refresh_form.validate():
                 refresh_form.populate_obj(self.app.currentuser)
diff --git a/rdiffweb/controller/tests/test_page_prefs_general.py b/rdiffweb/controller/tests/test_page_prefs_general.py
index 15b8ee415..be60757cf 100644
--- a/rdiffweb/controller/tests/test_page_prefs_general.py
+++ b/rdiffweb/controller/tests/test_page_prefs_general.py
@@ -80,11 +80,12 @@ class PagePrefGeneralTest(rdiffweb.test.WebCase):
         self.getPage(
             self.PREFS,
             method='POST',
             body={'action': 'set_profile_info', 'email': 'test@test.com', 'username': 'test'},
         )
-        self.assertStatus(200)
+        self.assertStatus(303)
+        self.getPage(self.PREFS)
         self.assertInBody("Profile updated successfully.")
         # Then database is updated with fullname
         user = UserObject.query.filter(UserObject.username == self.USERNAME).first()
         self.assertIsNotNone(user)
         self.assertEqual("test@test.com", user.email)
@@ -103,18 +104,20 @@ class PagePrefGeneralTest(rdiffweb.test.WebCase):
     )
     def test_change_fullname(self, new_fullname, expected_valid):
         # Given an authenticated user
         # When update the fullname
         self._set_profile_info("test@test.com", new_fullname)
-        self.assertStatus(200)
         if expected_valid:
+            self.assertStatus(303)
+            self.getPage(self.PREFS)
             self.assertInBody("Profile updated successfully.")
             # Then database is updated with fullname
             self.assertInBody(new_fullname)
             user = UserObject.query.filter(UserObject.username == self.USERNAME).first()
             self.assertEqual(new_fullname, user.fullname)
         else:
+            self.assertStatus(200)
             self.assertNotInBody("Profile updated successfully.")
 
     def test_change_fullname_method_get(self):
         # Given an authenticated user
         # When trying to update full name using GET method
@@ -137,11 +140,12 @@ class PagePrefGeneralTest(rdiffweb.test.WebCase):
         user = UserObject.query.filter(UserObject.username == self.USERNAME).first()
         self.assertEqual("", user.fullname)
 
     def test_change_email(self):
         self._set_profile_info("test@test.com")
-        self.assertStatus(200)
+        self.assertStatus(303)
+        self.getPage(self.PREFS)
         self.assertInBody("Profile updated successfully.")
 
     @parameterized.expand(
         [
             # Invalid
@@ -154,15 +158,17 @@ class PagePrefGeneralTest(rdiffweb.test.WebCase):
             ('test@test.com', True),
         ]
     )
     def test_change_email_with_invalid_email(self, new_email, expected_valid):
         self._set_profile_info(new_email)
-        self.assertStatus(200)
         if expected_valid:
+            self.assertStatus(303)
+            self.getPage(self.PREFS)
             self.assertInBody("Profile updated successfully.")
             self.assertNotInBody("Must be a valid email address.")
         else:
+            self.assertStatus(200)
             self.assertNotInBody("Profile updated successfully.")
             self.assertInBody("Must be a valid email address.")
 
     def test_change_email_with_too_long(self):
         self._set_profile_info(("test1" * 50) + "@test.com")
@@ -170,10 +176,14 @@ class PagePrefGeneralTest(rdiffweb.test.WebCase):
 
     def test_change_password(self):
         self.listener.user_password_changed.reset_mock()
         # When udating user's password
         self._set_password(self.PASSWORD, "pr3j5Dwi", "pr3j5Dwi")
+        # Then user is redirect to same page
+        self.assertStatus(303)
+        # Then the page return success message.
+        self.getPage(self.PREFS)
         self.assertInBody("Password updated successfully.")
         # Then a notification is raised
         self.listener.user_password_changed.assert_called_once()
 
     def test_change_password_with_wrong_confirmation(self):
@@ -206,10 +216,20 @@ class PagePrefGeneralTest(rdiffweb.test.WebCase):
         # Then a warning message is displayed on login page
         self.getPage('/login/')
         self.assertStatus(200)
         self.assertInBody('You were logged out because you entered the wrong password too many times.')
 
+    def test_change_password_with_same_value(self):
+        # Given a user with a password
+        self._set_password(self.PASSWORD, "pr3j5Dwi", "pr3j5Dwi")
+        self.assertStatus(303)
+        # When updating the pasword with the same password
+        self._set_password("pr3j5Dwi", "pr3j5Dwi", "pr3j5Dwi")
+        self.assertStatus(200)
+        # Then an error should be displayed
+        self.assertInBody("The new password must be different from the current password.")
+
     def test_change_password_method_get(self):
         # Given an authenticated user
         # Trying to update password with GET method
         self.getPage(self.PREFS + '?action=set_password&new=pr3j5Dwi&confirm=pr3j5Dwi&current=' + self.PASSWORD)
         # Then nothing happen

Vulnerable code:             flash(_("Wrong current password."), level='warning')
        else:
            # Clear number of attempts
            if CHANGE_PASSWORD_ATTEMPTS in cherrypy.session:
                del cherrypy.session[CHANGE_PASSWORD_ATTEMPTS]
            # If Valid, update password
            try:
                user.set_password(self.new.data)
                flash(_("Password updated successfully."), level='success')
            except ValueError as e:
                flash(str(e), level='warning')
                password_form.populate_obj(self.app.currentuser)
        self.assertStatus(200)
        self.assertStatus(200)
        self.assertStatus(200)
        self.assertStatus(200)
Fixed code: * Enforce password policy new password cannot be set as new password [CVE-2022-3376](https://nvd.nist.gov/vuln/detail/CVE-2022-3376)
    def validate_new(self, field):
        """
        Make sure new password if not equals to old password.
        """
        if self.new.data and self.new.data == self.current.data:
            raise ValueError(_('The new password must be different from the current password.'))

            self.current.errors = [_("Wrong current password.")]
            return False

        # Clear number of attempts
        if CHANGE_PASSWORD_ATTEMPTS in cherrypy.session:
            del cherrypy.session[CHANGE_PASSWORD_ATTEMPTS]

        try:
            user.set_password(self.new.data)
            return True
        except ValueError as e:
            self.new.errors = [str(e)]
            return False
                raise cherrypy.HTTPRedirect("")
                if password_form.populate_obj(self.app.currentuser):
                    flash(_("Password updated successfully."), level='success')
                    raise cherrypy.HTTPRedirect("")
        self.assertStatus(303)
        self.getPage(self.PREFS)
            self.assertStatus(303)
            self.getPage(self.PREFS)
            self.assertStatus(200)
        self.assertStatus(303)
        self.getPage(self.PREFS)
            self.assertStatus(303)
            self.getPage(self.PREFS)
            self.assertStatus(200)
        # Then user is redirect to same page
        self.assertStatus(303)
        # Then the page return success message.
        self.getPage(self.PREFS)
    def test_change_password_with_same_value(self):
        # Given a user with a password
        self._set_password(self.PASSWORD, "pr3j5Dwi", "pr3j5Dwi")
        self.assertStatus(303)
        # When updating the pasword with the same password
        self._set_password("pr3j5Dwi", "pr3j5Dwi", "pr3j5Dwi")
        self.assertStatus(200)
        # Then an error should be displayed
        self.assertInBody("The new password must be different from the current password.")