From fcd5fd8641503bd378e401bbc5b815a1efe8ccdb Mon Sep 17 00:00:00 2001
From: rlwilliamson-dev <123014229+rlwilliamson-dev@users.noreply.github.com>
Date: Tue, 2 Jun 2026 15:07:50 -0500
Subject: [PATCH] room-server: fix login when a password is set over a longer
 one
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

StrHelper::strncpy null-terminates but does NOT zero-pad the 16-byte
password buffer, so setting a shorter password over a longer previous
value (e.g. one inherited from a prior repeater config) leaves trailing
garbage. onAnonDataRecv's constant-time compare runs over the full
buffer width, so a correct password stopped matching — admin/guest
logins were silently rejected, or downgraded to a read-only guest when
allow_read_only was on (the login looked identical to read-only).

Fix: copy both stored passwords into zeroed buffers (up to strnlen)
before the constant-time compare, so the comparison reflects the actual
string while staying constant-time over the full width.

Hardware-verified on the kit: admin login with the correct password now
grants ADMIN (post + remote management), confirmed server-side via
get acl (perms 03).

Note: the repeater's handleLoginReq shares this latent issue.
---
 zephcore/app/RoomServerMesh.cpp | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/zephcore/app/RoomServerMesh.cpp b/zephcore/app/RoomServerMesh.cpp
index 83713d0..3cc7301 100644
--- a/zephcore/app/RoomServerMesh.cpp
+++ b/zephcore/app/RoomServerMesh.cpp
@@ -777,12 +777,24 @@ void RoomServerMesh::onAnonDataRecv(mesh::Packet* packet, const uint8_t* secret,
     if (client == nullptr) {
         /* Constant-time compare against both stored passwords.  Admin grants
          * ADMIN; the guest/room password grants READ_WRITE (so guests may
-         * post); allow_read_only downgrades any other login to GUEST. */
+         * post); allow_read_only downgrades any other login to GUEST.
+         *
+         * Zero-pad BOTH operands into cleared buffers first: the CLI's
+         * StrHelper::strncpy null-terminates but does NOT clear the rest of
+         * the 16-byte buffer, so a password set over a longer previous value
+         * leaves trailing garbage. Comparing the raw stored buffer full-width
+         * against the (zero-padded) received bytes would then fail to match a
+         * correct password. Copy only up to the NUL so the compare reflects
+         * the actual string while staying constant-time over the full width. */
         uint8_t received[sizeof(_prefs.password)] = {0};
+        uint8_t admin_pw[sizeof(_prefs.password)] = {0};
+        uint8_t guest_pw[sizeof(_prefs.guest_password)] = {0};
         size_t r_len = strnlen((const char*)&data[8], sizeof(received) - 1);
         memcpy(received, &data[8], r_len);
-        bool admin_match = mesh::Utils::constantTimeEqual(received, _prefs.password, sizeof(received));
-        bool guest_match = mesh::Utils::constantTimeEqual(received, _prefs.guest_password, sizeof(received));
+        memcpy(admin_pw, _prefs.password, strnlen(_prefs.password, sizeof(admin_pw) - 1));
+        memcpy(guest_pw, _prefs.guest_password, strnlen(_prefs.guest_password, sizeof(guest_pw) - 1));
+        bool admin_match = mesh::Utils::constantTimeEqual(received, admin_pw, sizeof(received));
+        bool guest_match = mesh::Utils::constantTimeEqual(received, guest_pw, sizeof(received));
 
         uint8_t perms;
         if (admin_match) {