Bladeren bron

LibCore: Use timing_safe_compare to compare password hash values

This is not strictly required, as we are comparing hashes, not the
password it self. However given this is generic code that could be
used anywhere in the system, it seems prudent to be cautious and
make sure we don't inadvertently leak any information about the hash
via timing attacks in future usages of `LibCore::Account`.

Reported-by: Jessica Hamilton <jessica.l.hamilton@gmail.com>
Brian Gianforcaro 3 jaren geleden
bovenliggende
commit
3ab2b90744
1 gewijzigde bestanden met toevoegingen van 3 en 1 verwijderingen
  1. 3 1
      Userland/Libraries/LibCore/Account.cpp

+ 3 - 1
Userland/Libraries/LibCore/Account.cpp

@@ -1,10 +1,12 @@
 /*
 /*
  * Copyright (c) 2020, Peter Elliott <pelliott@serenityos.org>
  * Copyright (c) 2020, Peter Elliott <pelliott@serenityos.org>
+ * Copyright (c) 2021-2022, Brian Gianforcaro <bgianf@serenityos.org>
  *
  *
  * SPDX-License-Identifier: BSD-2-Clause
  * SPDX-License-Identifier: BSD-2-Clause
  */
  */
 
 
 #include <AK/Base64.h>
 #include <AK/Base64.h>
+#include <AK/Memory.h>
 #include <AK/Random.h>
 #include <AK/Random.h>
 #include <AK/ScopeGuard.h>
 #include <AK/ScopeGuard.h>
 #include <LibCore/Account.h>
 #include <LibCore/Account.h>
@@ -134,7 +136,7 @@ bool Account::authenticate(SecretString const& password) const
 
 
     // FIXME: Use crypt_r if it can be built in lagom.
     // FIXME: Use crypt_r if it can be built in lagom.
     char* hash = crypt(password.characters(), m_password_hash.characters());
     char* hash = crypt(password.characters(), m_password_hash.characters());
-    return hash != nullptr && strcmp(hash, m_password_hash.characters()) == 0;
+    return hash != nullptr && AK::timing_safe_compare(hash, m_password_hash.characters(), m_password_hash.length());
 }
 }
 
 
 bool Account::login() const
 bool Account::login() const