room-server: fix login when a password is set over a longer one

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.
This commit is contained in:
rlwilliamson-dev
2026-06-02 15:07:50 -05:00
parent 58dbba7f3b
commit fcd5fd8641
+15 -3
View File
@@ -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) {