Skip to content

Potential fix for code scanning alert no. 8: Use of password hash with insufficient computational effort#18

Merged
Tanker187 merged 1 commit intomainfrom
alert-autofix-8
Feb 24, 2026
Merged

Potential fix for code scanning alert no. 8: Use of password hash with insufficient computational effort#18
Tanker187 merged 1 commit intomainfrom
alert-autofix-8

Conversation

@Tanker187
Copy link
Copy Markdown
Owner

Potential fix for https://github.com/Tanker187/firebase-admin-node/security/code-scanning/8

In general, the problem occurs because a password is hashed using a fast, single-iteration HMAC-SHA256, which does not provide the computational hardness required for password storage. To fix this, we should use a dedicated password hashing function, such as bcrypt, PBKDF2, scrypt, or Argon2. In Node’s standard library, crypto.pbkdf2/pbkdf2Sync is the readily available KDF; the file already imports bcrypt as well, but pbkdf2 will integrate more naturally with existing Buffer-based code and allows us to avoid changing importOptions semantics too much.

The single best fix here is to replace the direct HMAC-SHA256 computation on line 1951 with a PBKDF2-based derivation that uses the password and salt as separate parameters, with an adequate iteration count and key length. Since this is a test file and we must not alter behavior outside the shown snippet, we will:

  • Change the password hash computation from:
    • crypto.createHmac('sha256', currentHashKey).update(rawPassword + rawSalt).digest();
  • To a PBKDF2 call, e.g.:
    • crypto.pbkdf2Sync(rawPassword, rawSalt, 100000, 32, 'sha256');

This keeps passwordHash as a Buffer, matching the previous type, and remains compatible with the surrounding test logic (which only checks that the user can be imported and retrieved). All changes occur in test/integration/auth.spec.ts around the importUsers() should upload a user to the specified tenant test. No new imports are necessary, since crypto is already imported. We won’t modify importOptions.hash itself, as it’s not shown and may be used elsewhere in the tests.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

…h insufficient computational effort

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@Tanker187 Tanker187 self-assigned this Feb 24, 2026
@Tanker187 Tanker187 marked this pull request as ready for review February 24, 2026 21:46
@Tanker187 Tanker187 merged commit 1c6769c into main Feb 24, 2026
5 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant