From 69b74d9330e42fc91a9c7423d00a06cd6d3732bf Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Fri, 8 Sep 2023 08:57:56 -0400
Subject: [PATCH] Avoid temporary storage of sensitive information. (#16272)

During the UI auth process, avoid storing sensitive information
into the database.
---
 changelog.d/16272.bugfix          |  1 +
 synapse/rest/client/account.py    |  4 ++--
 tests/rest/client/test_account.py | 13 +++++++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)
 create mode 100644 changelog.d/16272.bugfix

diff --git a/changelog.d/16272.bugfix b/changelog.d/16272.bugfix
new file mode 100644
index 0000000000..afb22a999f
--- /dev/null
+++ b/changelog.d/16272.bugfix
@@ -0,0 +1 @@
+Avoid temporary storage of sensitive information.
diff --git a/synapse/rest/client/account.py b/synapse/rest/client/account.py
index 196b292890..49cd0805fd 100644
--- a/synapse/rest/client/account.py
+++ b/synapse/rest/client/account.py
@@ -186,7 +186,7 @@ class PasswordRestServlet(RestServlet):
                 params, session_id = await self.auth_handler.validate_user_via_ui_auth(
                     requester,
                     request,
-                    body.dict(exclude_unset=True),
+                    body.dict(exclude_unset=True, exclude={"new_password"}),
                     "modify your account password",
                 )
                 user_id = requester.user.to_string()
@@ -194,7 +194,7 @@ class PasswordRestServlet(RestServlet):
                 result, params, session_id = await self.auth_handler.check_ui_auth(
                     [[LoginType.EMAIL_IDENTITY]],
                     request,
-                    body.dict(exclude_unset=True),
+                    body.dict(exclude_unset=True, exclude={"new_password"}),
                     "modify your account password",
                 )
 
diff --git a/tests/rest/client/test_account.py b/tests/rest/client/test_account.py
index e9f495e206..4a0eca5b30 100644
--- a/tests/rest/client/test_account.py
+++ b/tests/rest/client/test_account.py
@@ -31,6 +31,7 @@ from synapse.rest import admin
 from synapse.rest.client import account, login, register, room
 from synapse.rest.synapse.client.password_reset import PasswordResetSubmitTokenResource
 from synapse.server import HomeServer
+from synapse.storage._base import db_to_json
 from synapse.types import JsonDict, UserID
 from synapse.util import Clock
 
@@ -134,6 +135,18 @@ class PasswordResetTestCase(unittest.HomeserverTestCase):
         # Assert we can't log in with the old password
         self.attempt_wrong_password_login("kermit", old_password)
 
+        # Check that the UI Auth information doesn't store the password in the database.
+        #
+        # Note that we don't have the UI Auth session ID, so just pull out the single
+        # row.
+        ui_auth_data = self.get_success(
+            self.store.db_pool.simple_select_one(
+                "ui_auth_sessions", keyvalues={}, retcols=("clientdict",)
+            )
+        )
+        client_dict = db_to_json(ui_auth_data["clientdict"])
+        self.assertNotIn("new_password", client_dict)
+
     @override_config({"rc_3pid_validation": {"burst_count": 3}})
     def test_ratelimit_by_email(self) -> None:
         """Test that we ratelimit /requestToken for the same email."""