From 794ae6cd60cc9c3d2bff81ea9ca704ba7c8fb6c0 Mon Sep 17 00:00:00 2001
From: MarkLee131 <kaixuan.li@ntu.edu.sg>
Date: Sat, 25 Apr 2026 10:39:13 +0800
Subject: [PATCH] User: use constant-time compare for MD5/SHA256/plain password
 paths

CString::Equals falls through to strcmp, which short-circuits on the first
differing byte. That leaks the length of the common prefix between the
stored hash (or plain password) and the attacker's guess via response
timing. Argon2id already uses argon2id_verify which is constant-time.

Add a small ConstantTimeEquals helper and use it for the legacy MD5,
SHA256 and plain HASH_NONE branches. No new dependency: the helper is
~10 lines and works on builds without OpenSSL.
---
 src/User.cpp | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/src/User.cpp b/src/User.cpp
index 42c7c5ec..bc438182 100644
--- a/src/User.cpp
+++ b/src/User.cpp
@@ -1054,6 +1054,25 @@ CConfig CUser::ToConfig() const {
     return config;
 }
 
+// Constant-time byte-wise compare of two strings. Returns true only if the
+// strings have the same length and the same contents. Runs in time
+// proportional to the shorter of the two lengths and does not short-circuit
+// on the first differing byte, so a remote attacker can't learn the stored
+// hash byte-by-byte via timing. Local helper to avoid a new OpenSSL
+// dependency for builds without libssl.
+static bool ConstantTimeEquals(const CString& a, const CString& b) {
+    if (a.length() != b.length()) {
+        return false;
+    }
+    unsigned char acc = 0;
+    const unsigned char* pa = reinterpret_cast<const unsigned char*>(a.data());
+    const unsigned char* pb = reinterpret_cast<const unsigned char*>(b.data());
+    for (size_t i = 0; i < a.length(); ++i) {
+        acc |= pa[i] ^ pb[i];
+    }
+    return acc == 0;
+}
+
 bool CUser::CheckPass(const CString& sPass) {
     if(AuthOnlyViaModule() || CZNC::Get().GetAuthOnlyViaModule()) {
         return false;
@@ -1063,11 +1082,13 @@ bool CUser::CheckPass(const CString& sPass) {
     bool bUpgrade = false;
     switch (m_eHashType) {
         case HASH_MD5:
-            bResult = m_sPass.Equals(CUtils::SaltedMD5Hash(sPass, m_sPassSalt));
+            bResult = ConstantTimeEquals(
+                m_sPass, CUtils::SaltedMD5Hash(sPass, m_sPassSalt));
             bUpgrade = true;
             break;
         case HASH_SHA256:
-            bResult = m_sPass.Equals(CUtils::SaltedSHA256Hash(sPass, m_sPassSalt));
+            bResult = ConstantTimeEquals(
+                m_sPass, CUtils::SaltedSHA256Hash(sPass, m_sPassSalt));
 #if ZNC_HAVE_ARGON
             bUpgrade = true;
 #endif
@@ -1082,7 +1103,7 @@ bool CUser::CheckPass(const CString& sPass) {
         case HASH_NONE:
             // Don't upgrade hash, since the only valid use case for plain are
             // manual tests, where it's simpler this way
-            return (sPass == m_sPass);
+            return ConstantTimeEquals(sPass, m_sPass);
     }
 
     if (bResult && bUpgrade) {