jeirmeister/Cockatrice
1
0
Fork 0
mirror of https://github.com/Cockatrice/Cockatrice.git synced 2026-11-05 01:23:24 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

[Utility] Budget scrypt's XY array in the derivation memory limit (#7712)

Browse source 
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 <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
BruebachL 2026-11-02 07:29:15 +01:00 • committed by GitHub
parent 719863e43d
commit b37c8b828f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 16 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

8
libcockatrice_utility/libcockatrice/utility/passwordhasher.cpp
View file

@ -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<quint64>(128) * n * r + static_cast<quint64>(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<quint64>(128) * n * r + static_cast<quint64>(128) * r * p + static_cast<quint64>(256) * r + 4096;
if (EVP_PBE_scrypt(passwordUtf8.constData(), passwordUtf8.size(),
reinterpret_cast<const unsigned char *>(salt.constData()), salt.size(), n, r, p, maxmem,
reinterpret_cast<unsigned char *>(key.data()), key.size()) != 1) {

11
tests/password_hash_test.cpp
View file

@ -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");