🛡️ CRITICAL: Fix SQL Injection Vulnerabilities

[MorDev1]

Security Issue: SQL Injection Vulnerabilities

Priority: CRITICAL

CVE Impact: Critical - Remote code execution potential
Security Risk: Complete database compromise and data breach

Problem Description

Multiple SQL injection vulnerabilities detected across the MCP server:

1. Dynamic SQL execution - Unsafe string concatenation in execute_sql
2. Unparameterized queries - Direct user input in SQL queries
3. Stored procedure vulnerabilities - RPC function security gaps
4. Second-order injection - Malicious data stored and later executed
5. Blind injection vulnerabilities - Information disclosure through timing

Current Vulnerabilities

Direct SQL Injection

• execute_sql tool accepts arbitrary SQL strings
• User management tools vulnerable to injection
• Migration tools allow malicious SQL execution
• Storage queries lack parameterization

Specific Vulnerable Areas

• src/tools/execute_sql.ts - Direct SQL execution
• src/tools/create_auth_user.ts - User creation injection
• src/tools/update_auth_user.ts - User update injection
• src/tools/delete_auth_user.ts - User deletion injection
• src/tools/apply_migration.ts - Migration SQL injection
• src/tools/list_storage_objects.ts - Storage query injection
• src/client/index.ts - RPC function creation vulnerability

Attack Vectors Identified

• Union-based: ' UNION SELECT * FROM auth.users --
• Boolean-based: ' OR 1=1 --
• Time-based: '; SELECT pg_sleep(5); --
• Stacked queries: '; DROP TABLE auth.users; --
• Second-order: Malicious data stored in user metadata

Required Implementation

1. Parameterized Queries

• Convert all dynamic SQL to parameterized queries
• Implement prepared statements for all database operations
• Add parameter validation and sanitization
• Replace string concatenation with parameter binding

2. Input Validation

• Implement strict input validation for all SQL-related inputs
• Add SQL keyword blacklisting
• Validate query structure and complexity
• Implement query timeout limits

3. SQL Execution Security

• Secure the execute_sql RPC function
• Add query whitelisting for safe operations
• Implement read-only query enforcement
• Add query complexity limits

4. Database Connection Security

• Use least-privilege database connections
• Implement connection-specific permissions
• Add query execution monitoring
• Implement prepared statement caching

Acceptance Criteria

• All SQL injection tests pass
• No dynamic SQL concatenation in codebase
• All database operations use parameterized queries
• Input validation prevents malicious SQL
• Query complexity limits are enforced
• Read-only queries are properly enforced
• Second-order injection is prevented
• Error messages don't leak database information

Testing Requirements

• Run SQL injection security test suite
• Test with OWASP SQLi payload database
• Validate against automated SQLi scanning tools
• Test second-order injection scenarios
• Verify blind injection protection
• Test union-based attack prevention

Implementation Steps

1. Phase 1: Audit all SQL execution paths
2. Phase 2: Implement parameterized queries
3. Phase 3: Add input validation and sanitization
4. Phase 4: Secure RPC function implementation
5. Phase 5: Add query monitoring and limits

Security Dependencies

• Requires authentication framework (🔐 CRITICAL: Implement Secure Authentication Framework #6)
• Blocks other security improvements until resolved
• Critical for MCP specification compliance

Files to Modify

• src/client/index.ts - Secure RPC function creation
• src/tools/execute_sql.ts - Parameterized query implementation
• src/tools/create_auth_user.ts - Secure user creation
• src/tools/update_auth_user.ts - Secure user updates
• src/tools/delete_auth_user.ts - Secure user deletion
• src/tools/apply_migration.ts - Safe migration execution
• src/tools/list_storage_objects.ts - Parameterized storage queries
• src/tools/utils.ts - Add SQL security utilities

Testing Commands

# Run SQL injection security tests
npm run test:security -- sql-injection.test.ts

# Run comprehensive security suite
npm run test:security

# Test with malicious payloads
npm run test:security -- --grep "SQL injection"

Security Improvements

Before (Vulnerable)

const query = `SELECT * FROM users WHERE id = '${userId}'`;
const result = await client.query(query);

After (Secure)

const query = 'SELECT * FROM users WHERE id = ';
const result = await client.query(query, [userId]);

RPC Function Security

CREATE OR REPLACE FUNCTION public.execute_sql(query text, params jsonb DEFAULT '[]')
RETURNS jsonb
LANGUAGE plpgsql
SECURITY DEFINER
AS $$
DECLARE
  result jsonb;
BEGIN
  -- Validate query against whitelist
  IF NOT is_query_safe(query) THEN
    RAISE EXCEPTION 'Query not allowed';
  END IF;
  
  -- Execute with parameters
  EXECUTE query USING params INTO result;
  RETURN result;
END;
$$;

References

• OWASP SQL Injection Prevention
• PostgreSQL Security Best Practices
• Node.js SQL Injection Prevention

Severity Justification

This is marked as CRITICAL because:

• SQL injection can lead to complete database compromise
• Allows arbitrary code execution on database server
• Can expose all sensitive data including credentials
• Enables privilege escalation and system takeover
• Required for basic security compliance