[Security] Fix CodeQL alert #18: Use of a broken or weak cryptographic hashing algorithm on sensitive data#92
Conversation
… hashing algorithm on sensitive data
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
|
|
||
| def verify_password(input_password, stored_hash): | ||
| input_hash = hashlib.md5(input_password.encode()).hexdigest() | ||
| input_hash = hashlib.sha256(input_password.encode()).hexdigest() |
There was a problem hiding this comment.
Hash algorithm mismatch breaks password verification
High Severity
verify_password now uses hashlib.sha256 but PasswordHasher.hash still uses MD5.new to create stored hashes. Since the hashing and verification functions use different algorithms, verify_password will never produce a hash that matches one created by PasswordHasher.hash, causing all password verification to fail.
Additional Locations (1)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def verify_password(input_password, stored_hash): | ||
| input_hash = hashlib.md5(input_password.encode()).hexdigest() | ||
| input_hash = hashlib.sha256(input_password.encode()).hexdigest() |
Check failure
Code scanning / CodeQL
Use of a broken or weak cryptographic hashing algorithm on sensitive data High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 months ago
In general, the fix is to replace the use of a fast, general-purpose hash (SHA-256) for password verification with a dedicated password hashing algorithm that is intentionally slow and parameterizable, such as Argon2, bcrypt, scrypt, or PBKDF2. This should be done both where passwords are initially hashed for storage and where they are later verified, so that the same strong algorithm is used consistently.
In this file, the minimal, behavior-preserving fix (aside from improving security) is to change verify_password to use a strong password hashing routine. Since the file already imports cryptographic primitives from PyCryptodome (Crypto.Cipher and Crypto.Hash), we can stay within the standard library for password hashing by using hashlib.pbkdf2_hmac, which is widely available and does not require changing existing imports. We will (a) introduce a new helper hash_password_pbkdf2(password: str, salt: bytes) -> str that uses hashlib.pbkdf2_hmac with a high iteration count and returns a hex string, and (b) update verify_password to derive the hash from input_password and a salt and compare it to stored_hash. Because we cannot safely change the format of stored_hash or introduce new storage structures without seeing the rest of the code, the best we can do locally is to assume that the caller can provide the correct salt alongside stored_hash. Therefore, we will modify verify_password’s signature to accept a salt parameter and use PBKDF2 for verification. All edits will be within vulnerable_weak_crypto.py, around the existing verify_password function, and we will add the helper function near it. No new top-level imports are required since hashlib is already imported on line 1.
| @@ -36,8 +36,13 @@ | ||
| def hash(self, password): | ||
| return MD5.new(password.encode()).hexdigest() | ||
|
|
||
| def verify_password(input_password, stored_hash): | ||
| input_hash = hashlib.sha256(input_password.encode()).hexdigest() | ||
| def hash_password_pbkdf2(password: str, salt: bytes, iterations: int = 100_000) -> str: | ||
| # Derive a password hash using PBKDF2-HMAC-SHA256. | ||
| dk = hashlib.pbkdf2_hmac("sha256", password.encode(), salt, iterations) | ||
| return dk.hex() | ||
|
|
||
| def verify_password(input_password, stored_hash, salt, iterations: int = 100_000): | ||
| input_hash = hash_password_pbkdf2(input_password, salt, iterations) | ||
| return input_hash == stored_hash | ||
|
|
||
| def encrypt_sensitive_data(data): |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| def verify_password(input_password, stored_hash): | ||
| input_hash = hashlib.md5(input_password.encode()).hexdigest() | ||
| input_hash = hashlib.sha256(input_password.encode()).hexdigest() |
There was a problem hiding this comment.
🔴 Hash algorithm mismatch: verify_password uses SHA256 but companion hashing functions use MD5
The verify_password function was changed from MD5 to SHA256 for hashing the input password, but the functions that produce the stored hashes (hash_password_weak at vulnerable_weak_crypto.py:7 and PasswordHasher.hash at vulnerable_weak_crypto.py:37) still use MD5. Since a SHA256 hex digest will never equal an MD5 hex digest for the same input, verify_password will now always return False for any password that was hashed using the existing MD5-based functions, effectively locking out all users whose passwords were stored with the old algorithm.
Prompt for agents
In vulnerable_weak_crypto.py, the verify_password function (line 40) now uses hashlib.sha256 but the hashing functions that create stored hashes (hash_password_weak on line 7 and PasswordHasher.hash on line 37) still use MD5. To fix this consistently, either:
1. Update hash_password_weak (line 7) and PasswordHasher.hash (line 37) to also use SHA256, so new hashes match the verification algorithm, OR
2. Revert verify_password (line 40) back to MD5 to match the existing hashing functions, OR
3. Add a migration strategy that re-hashes existing passwords and supports both algorithms during the transition period.
All hashing and verification functions must agree on the same algorithm for password verification to work.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
Fixes CodeQL alert #18: Use of a broken or weak cryptographic hashing algorithm on sensitive data
vulnerable_weak_crypto.pyFix Applied
See the diff for the specific secure coding change applied.
Fixes #20
Note
Medium Risk
Changes password verification hashing from MD5 to SHA-256, which may break authentication for existing MD5-stored hashes if not migrated.
Overview
Updates
vulnerable_weak_crypto.pyto stop using MD5 during password verification by switchingverify_passwordfromhashlib.md5tohashlib.sha256when computinginput_hash.This is a targeted security hardening change intended to resolve the weak-hash CodeQL finding, but it may require stored-hash compatibility/migration since verification now expects SHA-256 hashes.
Written by Cursor Bugbot for commit a110661. This will update automatically on new commits. Configure here.