Merge pull request #22 from QWED-AI/security/fixes

Security: Fix ReDoS vulnerability and Upgrade to HMAC-SHA256

Author: rahuldass19

src/qwed_new/auth/middleware.py

@@ -27,7 +27,7 @@ async def get_api_key(
     # Hash the provided key to compare with stored hash
     # The key format is qwed_live_<random>
     # We store the hash of the full key string
-    hashed_key = hashlib.sha256(api_key_header.encode()).hexdigest()
+    hashed_key = hash_api_key(api_key_header)
 
     # Query database for the key
     statement = select(ApiKey).where(

src/qwed_new/auth/security.py

@@ -65,13 +65,36 @@ def generate_api_key(prefix: str = "qwed_live") -> tuple[str, str]:
     plaintext_key = f"{prefix}_{random_part}"
 
     # Hash the key for storage
-    key_hash = hashlib.sha256(plaintext_key.encode()).hexdigest()
+    key_hash = hash_api_key(plaintext_key)
 
     return plaintext_key, key_hash
 
+
 def hash_api_key(api_key: str) -> str:
-    """Hash an API key for comparison."""
-    return hashlib.sha256(api_key.encode()).hexdigest()
+    """
+    Derive a hash for an API key using PBKDF2-HMAC-SHA256.
+
+    This is intentionally computationally expensive to make brute-force attacks
+    against stored API key hashes more difficult, while remaining deterministic
+    for lookup purposes.
+    """
+    # Derive a salt from SECRET_KEY; fall back to a constant development salt.
+    if isinstance(SECRET_KEY, str):
+        secret_bytes = SECRET_KEY.encode()
+    else:
+        secret_bytes = b"default_dev_salt" # Fallback if secret is somehow bytes or None
+
+    # Namespace the salt for API key hashing to avoid cross-protocol reuse.
+    salt = secret_bytes + b":qwed_api_key"
+
+    # Use PBKDF2-HMAC-SHA256 with a reasonable iteration count.
+    dk = hashlib.pbkdf2_hmac(
+        "sha256",
+        api_key.encode("utf-8"),
+        salt,
+        100_000,
+    )
+    return dk.hex()
 
 def mask_api_key(api_key: str) -> str:
     """

src/qwed_new/core/image_verifier.py

@@ -120,6 +120,17 @@ def verify_image(
                 "methods_used": [],
                 "engine": "ImageVerifier"
             }
+            
+        # Security: Prevent ReDoS attacks on regexes
+        if len(claim) > 500:
+            return {
+                "verdict": "INCONCLUSIVE",
+                "confidence": 0.0,
+                "reasoning": "Claim text too long (max 500 chars) - Security ReDoS protection",
+                "analysis": {},
+                "methods_used": [],
+                "engine": "ImageVerifier"
+            }
 
         methods_used = []
         analysis = {}

src/qwed_new/core/tenant_context.py

@@ -31,7 +31,7 @@ async def get_current_tenant(
     Returns the authenticated tenant context.
     """
     # 1. Hash the provided key
-    hashed_key = hashlib.sha256(x_api_key.encode()).hexdigest()
+    hashed_key = hash_api_key(x_api_key)
 
     # 2. Look up API key
     statement = select(ApiKey).where(ApiKey.key_hash == hashed_key, ApiKey.is_active == True)

tests/test_security_fixes.py

@@ -0,0 +1,43 @@
+from qwed_new.auth.security import hash_api_key
+from qwed_new.core.image_verifier import ImageVerifier
+
+def test_hash_api_key_pbkdf2():
+    """
+    Test that hash_api_key uses PBKDF2 (returns a hex string).
+    This ensures the new security logic is covered.
+    """
+    key = "qwed_live_test_key_12345"
+    hashed = hash_api_key(key)
+    
+    # Check it returns a string
+    assert isinstance(hashed, str)
+    # Check it's hex
+    assert all(c in '0123456789abcdef' for c in hashed)
+    # Length of SHA256 hex digest is 64 chars
+    assert len(hashed) == 64
+    
+    # Deterministic check
+    assert hash_api_key(key) == hashed
+
+def test_image_verifier_redos_protection():
+    """
+    Test that ImageVerifier rejects overly long claim strings (ReDoS protection).
+    """
+    verifier = ImageVerifier(use_vlm_fallback=False)
+    
+    # Create a dummy image (valid PNG header)
+    dummy_image = b'\x89PNG\r\n\x1a\n' + b'\x00' * 100
+    
+    # 1. Normal claim
+    normal_claim = "Is this a cat?"
+    result = verifier.verify_image(dummy_image, normal_claim)
+    # Should perform analysis (likely INCONCLUSIVE or fail elsewhere, but not ReDoS block)
+    assert result["verdict"] != "INCONCLUSIVE" or result["reasoning"] != "Claim text too long (max 500 chars) - Security ReDoS protection"
+
+    # 2. Long claim (ReDoS attack simulation)
+    long_claim = "a" * 600  # Exceeds 500 char limit
+    result_blocked = verifier.verify_image(dummy_image, long_claim)
+    
+    assert result_blocked["verdict"] == "INCONCLUSIVE"
+    assert "Claim text too long" in result_blocked["reasoning"]
+    assert result_blocked["confidence"] == 0.0