[Security] Fix CodeQL alert #17: Use of a broken or weak cryptographic hashing algorithm on sensitive data#91
Conversation
… hashing algorithm on sensitive data
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| def hash(self, password): | ||
| return MD5.new(password.encode()).hexdigest() | ||
| from Crypto.Hash import SHA256 | ||
| return SHA256.new(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 hashing with a dedicated password hashing algorithm that is computationally expensive and salted. The recommended options include Argon2, bcrypt, scrypt, or PBKDF2. Since we need to preserve the existing interface and functionality as much as possible, the best approach is to keep the PasswordHasher.hash method but change its internal implementation to call a strong password hashing primitive.
Within vulnerable_weak_crypto.py, we should modify the PasswordHasher.hash method (lines 35–38). Instead of importing and using Crypto.Hash.SHA256, we’ll import PasswordHasher from the argon2 package and use it to generate the password hash. This aligns with the background example and avoids changing other parts of the file. Specifically:
- Add a top-level import
from argon2 import PasswordHasher as Argon2PasswordHasher. - In the
PasswordHasher.hashmethod, instantiate anArgon2PasswordHasherand call its.hash(password)method, returning that value.
We’ll avoid altering any other functions, even though some also use weak crypto, because the specific CodeQL alert is about line 38.
| @@ -2,6 +2,7 @@ | ||
| import random | ||
| from Crypto.Cipher import DES, ARC2, Blowfish | ||
| from Crypto.Hash import MD5, SHA1 | ||
| from argon2 import PasswordHasher as Argon2PasswordHasher | ||
|
|
||
| def hash_password_weak(password): | ||
| return hashlib.md5(password.encode()).hexdigest() | ||
| @@ -34,8 +35,8 @@ | ||
|
|
||
| class PasswordHasher: | ||
| def hash(self, password): | ||
| from Crypto.Hash import SHA256 | ||
| return SHA256.new(password.encode()).hexdigest() | ||
| hasher = Argon2PasswordHasher() | ||
| return hasher.hash(password) | ||
|
|
||
| def verify_password(input_password, stored_hash): | ||
| input_hash = hashlib.md5(input_password.encode()).hexdigest() |
| @@ -1,4 +1,7 @@ | ||
| [[source]] | ||
| [packages] | ||
|
|
||
| argon2-cffi = "^25.1.0" | ||
| url = "https://pypi.org/simple" | ||
| verify_ssl = true | ||
| name = "pypi" |
| Package | Version | Security advisories |
| argon2-cffi (pypi) | 25.1.0 | None |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return SHA256.new(password.encode()).hexdigest() | ||
|
|
||
| def verify_password(input_password, stored_hash): | ||
| input_hash = hashlib.md5(input_password.encode()).hexdigest() |
There was a problem hiding this comment.
🔴 Hash algorithm mismatch: PasswordHasher.hash upgraded to SHA256 but verify_password still uses MD5
PasswordHasher.hash was changed from MD5 to SHA256 (vulnerable_weak_crypto.py:37-38), but the companion verify_password function at vulnerable_weak_crypto.py:40-42 still computes hashlib.md5(input_password.encode()).hexdigest(). Any password hashed with the updated PasswordHasher.hash() will produce a SHA256 digest, which will never match when checked by verify_password() since it computes an MD5 digest. This breaks password verification for any passwords stored using the new hashing method.
Prompt for agents
In vulnerable_weak_crypto.py, the verify_password function on line 40-42 still uses hashlib.md5 to compute the hash for comparison, but PasswordHasher.hash on line 36-38 was upgraded to use SHA256. Update verify_password to use SHA256 as well, e.g. change line 41 from:
input_hash = hashlib.md5(input_password.encode()).hexdigest()
to:
input_hash = hashlib.sha256(input_password.encode()).hexdigest()
Alternatively, have verify_password use the PasswordHasher class to ensure consistency. Also consider whether any existing stored hashes (created with the old MD5 algorithm) need a migration path.
Was this helpful? React with 👍 or 👎 to provide feedback.
![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 hash(self, password): | ||
| return MD5.new(password.encode()).hexdigest() | ||
| from Crypto.Hash import SHA256 | ||
| return SHA256.new(password.encode()).hexdigest() |
There was a problem hiding this comment.
Hash algorithm mismatch breaks password verification
High Severity
PasswordHasher.hash() was changed from MD5 to SHA256, but verify_password() still uses hashlib.md5 to compute the hash for comparison. Any password hashed with the updated PasswordHasher will never match when verified through verify_password, since a SHA256 digest will never equal an MD5 digest.
Additional Locations (1)
2 participants
Summary
Fixes CodeQL alert #17: 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 #19
Note
Medium Risk
Moderate risk because changing the hashing algorithm can break verification/compatibility for any passwords previously stored as MD5 hashes.
Overview
Updates
PasswordHasher.hashinvulnerable_weak_crypto.pyto useCrypto.Hash.SHA256instead ofCrypto.Hash.MD5, addressing the flagged weak hashing algorithm for password hashing.Written by Cursor Bugbot for commit f939fc1. This will update automatically on new commits. Configure here.