Skip to content

[Security] Fix CodeQL alert #17: Use of a broken or weak cryptographic hashing algorithm on sensitive data #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
colin-d-fried wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Security] Fix CodeQL alert #17: Use of a broken or weak cryptographic hashing algorithm on sensitive data #91

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion vulnerable_weak_crypto.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@

class PasswordHasher:
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

Sensitive data (password)
Loading
is used in a hashing algorithm (SHA256) that is insecure for password hashing, since it is not a computationally expensive hash function.

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.hash method, instantiate an Argon2PasswordHasher and 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.
Suggested changeset 2
vulnerable_weak_crypto.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/vulnerable_weak_crypto.py b/vulnerable_weak_crypto.py
--- a/vulnerable_weak_crypto.py
+++ b/vulnerable_weak_crypto.py
@@ -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()
EOF
@@ -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()
Pipfile
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/Pipfile b/Pipfile
--- a/Pipfile
+++ b/Pipfile
@@ -1,4 +1,7 @@
 [[source]]
+[packages]
+
+argon2-cffi = "^25.1.0"
 url = "https://pypi.org/simple"
 verify_ssl = true
 name = "pypi"
EOF
@@ -1,4 +1,7 @@
[[source]]
[packages]

argon2-cffi = "^25.1.0"
url = "https://pypi.org/simple"
verify_ssl = true
name = "pypi"
This fix introduces these dependencies
Package Version Security advisories
argon2-cffi (pypi) 25.1.0 None
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Copy Markdown

@cursor cursor Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web


def verify_password(input_password, stored_hash):
input_hash = hashlib.md5(input_password.encode()).hexdigest()
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 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.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Expand Down
Loading