Skip to content

Conversation

@ismailtsdln
Copy link
Member

Potential fix for https://github.com/ismailtsdln/GhidraInsight/security/code-scanning/5

In general, to fix this issue you should avoid using fast general-purpose hashes (like SHA‑256) for hashing secrets such as passwords or API keys for storage/verification. Instead, use a dedicated password hashing or key derivation function that is intentionally slow and salted, such as Argon2, scrypt, bcrypt, or PBKDF2 (via hashlib.pbkdf2_hmac in the standard library).

For this codebase, the minimal-impact fix is to change AuthManager.hash_api_key and AuthManager.verify_api_key so that:

  • hash_api_key derives a key from the API key using PBKDF2‑HMAC with SHA‑256, a sufficiently large iteration count, and a per-key random salt.
  • The function returns both the parameters and the derived key in a single encoded string (e.g., pbkdf2_sha256$<iterations>$<salt_hex>$<dk_hex>), so verification can reconstruct the same parameters.
  • verify_api_key parses that stored string, recomputes the PBKDF2 output, and compares using a constant-time comparison (hmac.compare_digest) to avoid timing side channels.

Concretely, in python-mcp/ghidrainsight/auth.py:

  • Replace the direct SHA‑256 hashing at line 154 with PBKDF2 using hashlib.pbkdf2_hmac, with a fixed iteration count (e.g., 100_000) and a random salt from secrets.token_bytes.
  • Change the docstring for hash_api_key to reflect that it’s using a strong password-hashing KDF (PBKDF2) rather than simple SHA‑256.
  • Update verify_api_key to:
    • Parse the stored hash format.
    • Recompute PBKDF2 with the same parameters.
    • Compare with hmac.compare_digest.
  • Add an import hmac at the top of auth.py since we need it for constant‑time comparison.

No changes are required in python-mcp/ghidrainsight/cli/__init__.py because the CLI simply calls AuthManager.hash_api_key(api_key) and prints the returned string; changing the internal hash format keeps behavior consistent from the CLI’s perspective.


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

…cryptographic hashing algorithm on sensitive data

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@gemini-code-assist
Copy link

Important

Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services.

@ismailtsdln ismailtsdln marked this pull request as ready for review January 5, 2026 17:10
@ismailtsdln ismailtsdln merged commit f37908f into main Jan 5, 2026
2 of 5 checks passed
@ismailtsdln ismailtsdln deleted the alert-autofix-5 branch January 5, 2026 17:23
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.

2 participants