From b37c8b828f382af67380019a804efa2825d6a0c0 Mon Sep 17 00:00:00 2001 From: BruebachL <44814898+BruebachL@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:29:15 +0100 Subject: [PATCH] [Utility] Budget scrypt's XY array in the derivation memory limit (#7712) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deriveKey passes maxmem = 128nr + 128rp + 4096 to EVP_PBE_scrypt, but scrypt also allocates XY at 256r bytes. costParamsAreSane accepts r up to 32, so any server asking for r >= 17 made derivation fail and the client report a login abort it could never recover from. Cover the XY array and add a regression test at the accepted-range boundary. Co-authored-by: Lukas Brübach --- .../libcockatrice/utility/passwordhasher.cpp | 8 +++++--- tests/password_hash_test.cpp | 11 +++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp index a7106aadc..8e876d5bc 100644 --- a/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp +++ b/libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp @@ -62,9 +62,11 @@ QByteArray PasswordHasher::deriveKey(const QString &password, const QByteArray & { QByteArray key(SCRYPT_VERIFIER_LENGTH, '\0'); const QByteArray passwordUtf8 = password.toUtf8(); - // EVP_PBE_scrypt aborts unless maxmem covers the required working memory, - // which is roughly 128 * n * r bytes (plus the small Salsa20/8 block array). - const auto maxmem = static_cast(128) * n * r + static_cast(128) * r * p + 4096; + // EVP_PBE_scrypt aborts unless maxmem covers the required working memory: V is 128 * n * + // r bytes, B is 128 * r * p, and XY is 256 * r. Under-budgeting XY makes OpenSSL fail + // for r >= 17, which costParamsAreSane still accepts. + const auto maxmem = + static_cast(128) * n * r + static_cast(128) * r * p + static_cast(256) * r + 4096; if (EVP_PBE_scrypt(passwordUtf8.constData(), passwordUtf8.size(), reinterpret_cast(salt.constData()), salt.size(), n, r, p, maxmem, reinterpret_cast(key.data()), key.size()) != 1) { diff --git a/tests/password_hash_test.cpp b/tests/password_hash_test.cpp index 99f2e1400..8e7eded72 100644 --- a/tests/password_hash_test.cpp +++ b/tests/password_hash_test.cpp @@ -52,6 +52,17 @@ TEST(PasswordHashTest, DeriveKeyMatchesKnownVector) ASSERT_EQ(derived.toHex(), expected.toHex()); } +TEST(PasswordHashTest, DeriveKeyCoversTheWholeAcceptedRangeOfR) +{ + // costParamsAreSane accepts r up to 32. The memory budget handed to OpenSSL has to cover + // scrypt's XY array (256 * r bytes) on top of V and B, or derivation fails for r >= 17. + for (const int r : {17, 32}) { + const QByteArray derived = PasswordHasher::deriveKey("password", QByteArray("NaCl"), 1024, r, 1); + ASSERT_FALSE(derived.isEmpty()) << "deriveKey failed for accepted r=" << r; + ASSERT_EQ(derived.size(), SCRYPT_VERIFIER_LENGTH); + } +} + TEST(PasswordHashTest, PasswordVerifierRoundTrip) { const QString stored = PasswordHasher::generatePasswordVerifier("hunter2");