Skip to content

Commit

Permalink
Fix for buffer overflow issue #728 (#729)
Browse files Browse the repository at this point in the history
* #728 Buffer overflow error

* Make sure old long passwords work as well
  • Loading branch information
tanabi authored Apr 12, 2024
1 parent 71e343f commit b8de02f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 22 deletions.
27 changes: 6 additions & 21 deletions src/fbmath.c
Original file line number Diff line number Diff line change
Expand Up @@ -584,26 +584,6 @@ rnd(void *buffer)
*
*********************************************************************/

#ifdef USE_SSL
static void
PBKDF2_HMAC_SHA_512(const char* pass, const unsigned char* salt,
int32_t iterations, uint32_t outputBytes,
char* hexResult)
{
unsigned int i;
unsigned char* digest;

digest = (unsigned char*)malloc(outputBytes);

PKCS5_PBKDF2_HMAC(pass, strlen(pass), salt, strlen(salt), iterations,
EVP_sha512(), outputBytes, digest);
for (i = 0; i < outputBytes; i++)
sprintf(hexResult + (i * 2), "%02x", 255 & digest[i]);

free(digest);
}
#endif

/**
* Generate a PBKDF2 password hash with the given password and salt.
*
Expand Down Expand Up @@ -672,7 +652,12 @@ pbkdf2_hash(const char* password, int password_len, const char* salt,
PKCS5_PBKDF2_HMAC(password, password_len, salt, salt_len, 1000,
EVP_sha512(), digest_len, digest);

for (i = 0; i < digest_len; i++) {
/*
* The -1 here should avoid a buffer overflow as otherwise this will
* get to be exactly the same size as buffer with no room for the
* null.
*/
for (i = 0; i < (digest_len - 1); i++) {
sprintf(buffer + salt_len + 4 + (i * 2), "%02x", 255 & digest[i]);
}

Expand Down
12 changes: 11 additions & 1 deletion src/player.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,17 @@ check_password(dbref player, const char *password)
}
}

if (!strcmp(pword, processed))
/*
* There was a bug where the password hash was causing a buffer
* overflow. Some compilers apparently cover this up or smooth
* this over in some fashion which means it is an inconsistent
* overflow.
*
* By matching by the length of 'processed', we'll be able to
* support any old "too long" hashes that may have slipped into
* the system.
*/
if (!strncmp(pword, processed, strlen(processed)))
return 1;

return 0;
Expand Down

0 comments on commit b8de02f

Please sign in to comment.