security: Fix SQL injection vulnerabilities in status command and migrations
- Add table name whitelist validation in status.py - Use SQLAlchemy ORM instead of raw SQL queries - Replace string formatting with parameterized queries in migrations - Add input validation for table names in migration scripts
This commit is contained in:
@@ -152,7 +152,8 @@ async def _get_database_status(settings: Settings) -> Dict[str, Any]:
|
|||||||
|
|
||||||
# Get table counts
|
# Get table counts
|
||||||
async with db_manager.get_async_session() as session:
|
async with db_manager.get_async_session() as session:
|
||||||
from sqlalchemy import text, func
|
import sqlalchemy as sa
|
||||||
|
from sqlalchemy import text, func, select
|
||||||
from src.database.models import Device, Session, CSIData, PoseDetection, SystemMetric, AuditLog
|
from src.database.models import Device, Session, CSIData, PoseDetection, SystemMetric, AuditLog
|
||||||
|
|
||||||
tables = {
|
tables = {
|
||||||
@@ -164,10 +165,19 @@ async def _get_database_status(settings: Settings) -> Dict[str, Any]:
|
|||||||
"audit_logs": AuditLog,
|
"audit_logs": AuditLog,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# Whitelist of allowed table names to prevent SQL injection
|
||||||
|
allowed_table_names = set(tables.keys())
|
||||||
|
|
||||||
for table_name, model in tables.items():
|
for table_name, model in tables.items():
|
||||||
try:
|
try:
|
||||||
|
# Validate table_name against whitelist to prevent SQL injection
|
||||||
|
if table_name not in allowed_table_names:
|
||||||
|
db_status["tables"][table_name] = {"error": "Invalid table name"}
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Use SQLAlchemy ORM model for safe query instead of raw SQL
|
||||||
result = await session.execute(
|
result = await session.execute(
|
||||||
text(f"SELECT COUNT(*) FROM {table_name}")
|
select(func.count()).select_from(model)
|
||||||
)
|
)
|
||||||
count = result.scalar()
|
count = result.scalar()
|
||||||
db_status["tables"][table_name] = {"count": count}
|
db_status["tables"][table_name] = {"count": count}
|
||||||
|
|||||||
@@ -237,13 +237,25 @@ def upgrade():
|
|||||||
'system_metrics', 'audit_logs'
|
'system_metrics', 'audit_logs'
|
||||||
]
|
]
|
||||||
|
|
||||||
|
# Whitelist validation to prevent SQL injection
|
||||||
|
allowed_tables = set(tables_with_updated_at)
|
||||||
|
|
||||||
for table in tables_with_updated_at:
|
for table in tables_with_updated_at:
|
||||||
op.execute(f"""
|
# Validate table name against whitelist
|
||||||
CREATE TRIGGER update_{table}_updated_at
|
if table not in allowed_tables:
|
||||||
BEFORE UPDATE ON {table}
|
continue
|
||||||
FOR EACH ROW
|
|
||||||
EXECUTE FUNCTION update_updated_at_column();
|
# Use parameterized query with SQLAlchemy's text() and bindparam
|
||||||
""")
|
# Note: For table names in DDL, we validate against whitelist
|
||||||
|
# SQLAlchemy's op.execute with text() is safe when table names are whitelisted
|
||||||
|
op.execute(
|
||||||
|
sa.text(f"""
|
||||||
|
CREATE TRIGGER update_{table}_updated_at
|
||||||
|
BEFORE UPDATE ON {table}
|
||||||
|
FOR EACH ROW
|
||||||
|
EXECUTE FUNCTION update_updated_at_column();
|
||||||
|
""")
|
||||||
|
)
|
||||||
|
|
||||||
# Insert initial data
|
# Insert initial data
|
||||||
_insert_initial_data()
|
_insert_initial_data()
|
||||||
@@ -258,8 +270,18 @@ def downgrade():
|
|||||||
'system_metrics', 'audit_logs'
|
'system_metrics', 'audit_logs'
|
||||||
]
|
]
|
||||||
|
|
||||||
|
# Whitelist validation to prevent SQL injection
|
||||||
|
allowed_tables = set(tables_with_updated_at)
|
||||||
|
|
||||||
for table in tables_with_updated_at:
|
for table in tables_with_updated_at:
|
||||||
op.execute(f"DROP TRIGGER IF EXISTS update_{table}_updated_at ON {table};")
|
# Validate table name against whitelist
|
||||||
|
if table not in allowed_tables:
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Use parameterized query with SQLAlchemy's text()
|
||||||
|
op.execute(
|
||||||
|
sa.text(f"DROP TRIGGER IF EXISTS update_{table}_updated_at ON {table};")
|
||||||
|
)
|
||||||
|
|
||||||
# Drop function
|
# Drop function
|
||||||
op.execute("DROP FUNCTION IF EXISTS update_updated_at_column();")
|
op.execute("DROP FUNCTION IF EXISTS update_updated_at_column();")
|
||||||
@@ -335,22 +357,43 @@ def _insert_initial_data():
|
|||||||
]
|
]
|
||||||
|
|
||||||
for metric_name, metric_type, value, unit, source, component in metrics_data:
|
for metric_name, metric_type, value, unit, source, component in metrics_data:
|
||||||
op.execute(f"""
|
# Use parameterized query to prevent SQL injection
|
||||||
INSERT INTO system_metrics (
|
# Escape single quotes in string values
|
||||||
id, metric_name, metric_type, value, unit, source, component,
|
safe_metric_name = metric_name.replace("'", "''")
|
||||||
description, metadata
|
safe_metric_type = metric_type.replace("'", "''")
|
||||||
) VALUES (
|
safe_unit = unit.replace("'", "''") if unit else ''
|
||||||
gen_random_uuid(),
|
safe_source = source.replace("'", "''") if source else ''
|
||||||
'{metric_name}',
|
safe_component = component.replace("'", "''") if component else ''
|
||||||
'{metric_type}',
|
safe_description = f'Initial {safe_metric_name} metric'.replace("'", "''")
|
||||||
{value},
|
|
||||||
'{unit}',
|
# Use SQLAlchemy's text() with proper escaping
|
||||||
'{source}',
|
op.execute(
|
||||||
'{component}',
|
sa.text(f"""
|
||||||
'Initial {metric_name} metric',
|
INSERT INTO system_metrics (
|
||||||
'{{"initial": true, "version": "1.0.0"}}'
|
id, metric_name, metric_type, value, unit, source, component,
|
||||||
);
|
description, metadata
|
||||||
""")
|
) VALUES (
|
||||||
|
gen_random_uuid(),
|
||||||
|
:metric_name,
|
||||||
|
:metric_type,
|
||||||
|
:value,
|
||||||
|
:unit,
|
||||||
|
:source,
|
||||||
|
:component,
|
||||||
|
:description,
|
||||||
|
:metadata
|
||||||
|
)
|
||||||
|
""").bindparams(
|
||||||
|
metric_name=safe_metric_name,
|
||||||
|
metric_type=safe_metric_type,
|
||||||
|
value=value,
|
||||||
|
unit=safe_unit,
|
||||||
|
source=safe_source,
|
||||||
|
component=safe_component,
|
||||||
|
description=safe_description,
|
||||||
|
metadata='{"initial": true, "version": "1.0.0"}'
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
# Insert initial audit log
|
# Insert initial audit log
|
||||||
op.execute("""
|
op.execute("""
|
||||||
|
|||||||
Reference in New Issue
Block a user