diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 4bd30c3..04c694f 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -19,3 +19,8 @@ **Prevention:** 1. Always convert `pathlib.Path` objects to absolute strings using `str(path.absolute())` before passing them as arguments to `subprocess.run`. 2. Absolute paths always begin with a directory separator (`/` on Unix) or a drive letter (`C:\` on Windows), guaranteeing the command-line tool parses them as file paths rather than flags or options. + +## 2024-05-30 - SQL Injection via Dictionary Keys in Dynamic SQL Updates +**Vulnerability:** In `metadata_generator.py:476`, dynamic SQL `INSERT` statements were constructed directly from dictionary keys representing file metadata (`metadata.keys()`). If an attacker could inject arbitrary keys into the metadata dictionary, they could perform SQL injection, breaking out of the statement and executing arbitrary SQL (e.g., `DROP TABLE`). +**Learning:** When generating SQL dynamically from dictionary keys, traditional parameterization (`?`) only protects the values, not the column names. Without validating the keys against a schema allowlist, any user-controlled dictionary input can lead to critical SQL injection vulnerabilities. +**Prevention:** Always validate dynamic column names against an explicit schema allowlist. Use `PRAGMA table_info` to fetch allowed columns from the database dynamically and verify that dictionary keys are both in the allowlist and pass Python's `str.isidentifier()` check before constructing the query. diff --git a/metadata_generator.py b/metadata_generator.py index ffa9d33..dae0bb8 100644 --- a/metadata_generator.py +++ b/metadata_generator.py @@ -231,9 +231,12 @@ def _migrate_database_schema(self, conn): migration_count = 0 for col_name, col_type in gdrive_columns.items(): if col_name not in existing_columns: - conn.execute(f"ALTER TABLE file_metadata ADD COLUMN {col_name} {col_type}") - migration_count += 1 - print(f" ✅ Added column: {col_name}") + if col_name.isidentifier(): + conn.execute(f"ALTER TABLE file_metadata ADD COLUMN {col_name} {col_type}") + migration_count += 1 + print(f" ✅ Added column: {col_name}") + else: + print(f" ⚠️ Skipping invalid column name: {col_name}") # Step 6: Commit all changes atomically conn.commit() @@ -467,9 +470,21 @@ def save_file_metadata(self, metadata: Dict[str, Any]) -> bool: try: with sqlite3.connect(self.db_path) as conn: + # Get valid columns from schema to prevent SQL injection + cursor = conn.execute("PRAGMA table_info(file_metadata)") + allowed_columns = {row[1] for row in cursor.fetchall()} + + safe_metadata = {} + for k, v in metadata.items(): + if k in allowed_columns and k.isidentifier(): + safe_metadata[k] = v + + if not safe_metadata: + return False + # Convert to database format - columns = list(metadata.keys()) - values = list(metadata.values()) + columns = list(safe_metadata.keys()) + values = list(safe_metadata.values()) placeholders = ', '.join(['?' for _ in values]) column_names = ', '.join(columns)