mysql-mcp-webui

# Security Audit Report - MySQL MCP WebUI **Audit Date:** 2025-11-06 **Auditor:** Claude Code (Automated Security Review) **Codebase Version:** Current (claude/security-audit-db-011CUrPZaKoyWXxitYf5Nkct branch) ## Executive Summary This security audit evaluated the MySQL MCP WebUI application for vulnerabilities that could lead to unauthorized database access or data exposure. The audit identified **3 CRITICAL**, **2 HIGH**, **3 MEDIUM**, and **4 LOW** severity issues that require attention. ### Risk Overview - **CRITICAL (3)**: SQL Injection vulnerabilities, timing attack vulnerability in API key verification - **HIGH (2)**: Information disclosure in logs, weak password requirements - **MEDIUM (3)**: Cookie security in development, CORS configuration, error message exposure - **LOW (4)**: Console logging, default credentials, master key storage, session timeout configuration --- ## CRITICAL Severity Issues ### 1. SQL Injection in Browse Endpoints **Severity:** CRITICAL **CWE:** CWE-89 (SQL Injection) **Location:** `server/src/api/routes/browse.ts` #### Description Multiple endpoints in the browse API construct SQL queries using string interpolation with user-controlled table names, making them vulnerable to SQL injection attacks. #### Vulnerable Code ```typescript // Line 38 `SELECT '${t.name.replace(/'/g, "''")}' as table_name, COUNT(*) as row_count FROM \`${t.name}\`` // Line 95 `DESCRIBE \`${tableName}\`` // Line 125 `SELECT COUNT(*) as total FROM \`${tableName}\`` // Line 132 `SELECT * FROM \`${tableName}\` LIMIT ${pageSize} OFFSET ${offset}` // Line 181 - MOST VULNERABLE WHERE table_schema = DATABASE() AND table_name = '${tableName}' // Line 212 `SHOW INDEX FROM \`${tableName}\`` ``` #### Impact An attacker could: - Execute arbitrary SQL queries - Access data from other tables - Bypass permission controls - Potentially modify or delete data - Extract sensitive information from the database #### Example Exploit ```http GET /api/browse/tables/users` UNION SELECT password_hash FROM users WHERE '1'='1/structure ``` #### Recommendation **IMMEDIATE ACTION REQUIRED:** 1. Use parameterized queries for all database operations 2. Validate table names against the actual list of tables before using them 3. Escape special characters properly, including backticks 4. Implement input validation with a whitelist approach **Example Fix:** ```typescript // Validate table name exists first const validTables = await queryExecutor.executeQuery('SHOW TABLES'); const tableNames = validTables.rows.map(row => Object.values(row)[0]); if (!tableNames.includes(tableName)) { throw new Error('Invalid table name'); } // Then use parameterized queries or properly escaped identifiers // MySQL doesn't support parameterized identifiers, so validate strictly const escapedTableName = tableName.replace(/[^\w]/g, ''); // Only allow alphanumeric const result = await queryExecutor.executeQuery(`DESCRIBE \`${escapedTableName}\``); ``` --- ### 2. Timing Attack Vulnerability in API Key Verification **Severity:** CRITICAL **CWE:** CWE-208 (Observable Timing Discrepancy) **Location:** `server/src/db/database-manager.ts:263-280` #### Description The API key verification uses a simple database lookup without constant-time comparison. While `constantTimeCompare` exists in `crypto.ts:101-110`, it is NOT used for API key verification, making the system vulnerable to timing attacks. #### Vulnerable Code ```typescript verifyApiKey(key: string): ApiKey | null { const stmt = this.db.prepare(` SELECT id, name, key, created_at, last_used_at, is_active FROM api_keys WHERE key = ? AND is_active = 1 `); const row = stmt.get(key) as any; // ⚠️ Direct comparison, timing attack possible if (!row) return null; // ... } ``` #### Impact An attacker could: - Use timing differences to brute-force API keys character by character - Significantly reduce the search space for valid keys - Gain unauthorized access to the system #### Recommendation **IMMEDIATE ACTION REQUIRED:** 1. Fetch all active API keys and compare each using `constantTimeCompare` 2. Maintain constant execution time regardless of match success 3. Consider hashing API keys at rest (similar to passwords) **Example Fix:** ```typescript import { constantTimeCompare } from '../config/crypto.js'; verifyApiKey(key: string): ApiKey | null { // Get all active API keys const stmt = this.db.prepare(` SELECT id, name, key, created_at, last_used_at, is_active FROM api_keys WHERE is_active = 1 `); const rows = stmt.all() as ApiKey[]; let matchedKey: ApiKey | null = null; // Compare all keys in constant time to prevent timing attacks for (const row of rows) { if (constantTimeCompare(row.key, key)) { matchedKey = row; // Don't break early - continue to maintain constant time } } if (matchedKey) { this.updateApiKeyLastUsed(matchedKey.id); } return matchedKey; } ``` --- ### 3. Insufficient Input Validation in Query Execution **Severity:** CRITICAL **CWE:** CWE-20 (Improper Input Validation) **Location:** `server/src/api/routes/browse.ts:132` #### Description The pagination parameters `pageSize` and `offset` are directly interpolated into SQL queries without proper validation or use of parameterized queries. #### Vulnerable Code ```typescript const pageSize = parseInt(req.query.pageSize as string) || 50; const offset = (page - 1) * pageSize; const dataResult = await queryExecutor.executeQuery( `SELECT * FROM \`${tableName}\` LIMIT ${pageSize} OFFSET ${offset}` ); ``` #### Impact While parseInt provides some protection, this pattern is dangerous: - Could lead to unexpected behavior with malformed input - Bypasses the permission validation system by using raw queries - Sets a bad precedent for other developers #### Recommendation 1. Use proper bounds checking on numeric inputs 2. Define maximum limits for pagination 3. Consider using parameterized queries where possible --- ## HIGH Severity Issues ### 4. Sensitive Data Logged in Request Logs **Severity:** HIGH **CWE:** CWE-532 (Insertion of Sensitive Information into Log File) **Location:** `server/src/api/middleware/logging.ts:74-82`, `server/src/db/database-manager.ts` #### Description The logging middleware captures both request and response bodies, which may contain: - Database passwords (from connection creation/update) - API keys - SQL queries with sensitive data - User passwords (during user creation) - MySQL query results with sensitive customer data #### Vulnerable Code ```typescript dbManager.logRequest( req.apiKeyId || null, req.path, req.method, req.body, // ⚠️ May contain passwords, API keys body, // ⚠️ May contain sensitive query results res.statusCode, duration, req.user?.userId ); ``` #### Impact - Sensitive data stored in plaintext in SQLite database - Accessible to anyone with database access - Not automatically cleaned up - Could expose passwords, API keys, and sensitive customer data #### Recommendation **ACTION REQUIRED:** 1. Implement request/response sanitization before logging 2. Redact sensitive fields like passwords, keys, tokens 3. Limit the size of logged bodies 4. Implement automatic log rotation/cleanup 5. Add encryption for stored logs **Example Fix:** ```typescript function sanitizeForLogging(data: any): any { if (!data) return data; const sensitiveKeys = ['password', 'apiKey', 'token', 'key', 'secret']; const sanitized = JSON.parse(JSON.stringify(data)); function redactSensitive(obj: any) { for (const key in obj) { if (sensitiveKeys.some(sk => key.toLowerCase().includes(sk))) { obj[key] = '[REDACTED]'; } else if (typeof obj[key] === 'object') { redactSensitive(obj[key]); } } } redactSensitive(sanitized); return sanitized; } // Use in logging dbManager.logRequest( req.apiKeyId || null, req.path, req.method, sanitizeForLogging(req.body), sanitizeForLogging(body), res.statusCode, duration, req.user?.userId ); ``` --- ### 5. Weak Password Requirements **Severity:** HIGH **CWE:** CWE-521 (Weak Password Requirements) **Location:** `server/src/api/routes/auth.ts:21` #### Description Password requirements are minimal - only 4 characters minimum. #### Vulnerable Code ```typescript const ChangePasswordSchema = z.object({ currentPassword: z.string().optional(), newPassword: z.string().min(4, 'Password must be at least 4 characters'), // ⚠️ Too weak confirmPassword: z.string().min(1), }); ``` #### Impact - Passwords like "1234" or "aaaa" are accepted - Vulnerable to brute force attacks - No complexity requirements - No password strength validation #### Recommendation **ACTION REQUIRED:** Implement stronger password requirements: ```typescript const ChangePasswordSchema = z.object({ currentPassword: z.string().optional(), newPassword: z.string() .min(12, 'Password must be at least 12 characters') .regex(/[A-Z]/, 'Password must contain at least one uppercase letter') .regex(/[a-z]/, 'Password must contain at least one lowercase letter') .regex(/[0-9]/, 'Password must contain at least one number') .regex(/[^A-Za-z0-9]/, 'Password must contain at least one special character'), confirmPassword: z.string().min(1), }); ``` --- ## MEDIUM Severity Issues ### 6. Cookie Security in Development Mode **Severity:** MEDIUM **CWE:** CWE-614 (Sensitive Cookie in HTTPS Session Without 'Secure' Attribute) **Location:** `server/src/api/routes/auth.ts:73-78` #### Description JWT cookies are not marked as `secure` in development mode, allowing them to be transmitted over HTTP. #### Vulnerable Code ```typescript res.cookie('auth_token', token, { httpOnly: true, secure: process.env.NODE_ENV === 'production', // ⚠️ Not secure in dev sameSite: 'strict', maxAge: 7 * 24 * 60 * 60 * 1000, }); ``` #### Impact - JWT tokens could be intercepted over unencrypted connections in development - Developers may accidentally deploy development config to production - MITM attacks possible in development environments #### Recommendation Consider always using `secure: true` and running development with HTTPS, or add clear warnings when HTTPS is not enabled. --- ### 7. Overly Permissive CORS in Development **Severity:** MEDIUM **CWE:** CWE-942 (Overly Permissive Cross-domain Whitelist) **Location:** `server/src/http-server.ts:30-35` #### Description CORS is configured to allow all origins in development mode. #### Vulnerable Code ```typescript app.use( cors({ origin: process.env.NODE_ENV === 'production' ? false : true, // ⚠️ Allows all in dev credentials: true, }) ); ``` #### Impact - Any website can make requests to the API in development - Credentials are included in cross-origin requests - Could lead to CSRF attacks during development #### Recommendation Specify allowed origins explicitly even in development: ```typescript app.use( cors({ origin: process.env.NODE_ENV === 'production' ? false : ['http://localhost:5173', 'http://localhost:9274'], credentials: true, }) ); ``` --- ### 8. Error Messages Expose Internal Details **Severity:** MEDIUM **CWE:** CWE-209 (Generation of Error Message Containing Sensitive Information) **Location:** Multiple locations (error handlers throughout codebase) #### Description Error messages in development mode expose internal details like stack traces and file paths. #### Impact - Attackers gain information about internal structure - File paths and technology stack revealed - Aids in targeted attacks #### Recommendation 1. Create standardized error responses 2. Log detailed errors server-side only 3. Return generic messages to clients 4. Implement error code system --- ## LOW Severity Issues ### 9. Extensive Console Logging **Severity:** LOW **CWE:** CWE-532 (Information Exposure Through Log Files) **Location:** Multiple files (14 files with console.log/error/warn) #### Description Extensive use of console logging throughout the application, including: - API key exposure: "Created default API key: {key}" - Token information - Internal state details #### Impact - Sensitive information in server logs - Production logs may contain secrets - Logs accessible to operators #### Recommendation 1. Use a proper logging library (winston, pino) 2. Implement log levels and filtering 3. Redact sensitive information from logs 4. Never log complete API keys or tokens --- ### 10. Default Credentials **Severity:** LOW **CWE:** CWE-798 (Use of Hard-coded Credentials) **Location:** `server/src/db/database-manager.ts:129-134` #### Description Default admin credentials (admin/admin) are created on first run. While forced password change is implemented, this is still a security concern. #### Impact - Window of vulnerability before password change - Well-known default credentials - Automated attacks may succeed #### Recommendation 1. Consider generating random default password 2. Display password only once during setup 3. Force immediate password change before any other actions 4. Add account lockout after failed login attempts --- ### 11. Master Key Storage in Database **Severity:** LOW **CWE:** CWE-320 (Key Management Errors) **Location:** `server/src/config/master-key.ts` #### Description The master encryption key used to encrypt database passwords is stored in the same SQLite database. #### Impact - If SQLite database is compromised, all MySQL passwords can be decrypted - Single point of failure - No key rotation mechanism #### Recommendation 1. Store master key in environment variable or external key management system 2. Consider using a hardware security module (HSM) for production 3. Implement key rotation procedures 4. Use different keys for different environments --- ### 12. Long Session Timeout **Severity:** LOW **CWE:** CWE-613 (Insufficient Session Expiration) **Location:** `server/src/mcp/session-manager.ts:17` #### Description HTTP sessions timeout after 30 minutes of inactivity, which is relatively long for a system managing sensitive database access. #### Impact - Prolonged access if session is compromised - Increased window for session hijacking #### Recommendation Consider reducing to 10-15 minutes or making it configurable based on security requirements. --- ## Positive Security Findings The following security practices are implemented well: ✅ **Password Hashing**: Bcrypt with 10 salt rounds ✅ **MySQL Password Encryption**: AES-256-GCM with proper IV and salt ✅ **httpOnly Cookies**: JWT tokens not accessible via JavaScript ✅ **SameSite Cookies**: CSRF protection via SameSite=strict ✅ **Permission System**: Granular per-database, per-operation permissions ✅ **SQL Parser**: Uses node-sql-parser for query type detection ✅ **Rate Limiting**: Configurable rate limiting on all endpoints ✅ **No Localhost Bypass**: Authentication always required ✅ **Prepared Statements**: Used for SQLite operations ✅ **HTTPS Support**: Optional TLS/SSL configuration ✅ **Forced Password Change**: Default admin account requires password change --- ## Compliance Considerations ### GDPR / Data Protection - **Concern**: Request logs may contain personal data - **Recommendation**: Implement data retention policies and automatic deletion - **Recommendation**: Add privacy notice about logging - **Recommendation**: Implement data export/deletion functionality ### PCI-DSS (if applicable) - **Concern**: Password requirements don't meet PCI standards (8+ characters, complexity) - **Concern**: No account lockout mechanism - **Concern**: No audit trail for administrative actions --- ## Priority Recommendations ### Immediate (Within 24 hours) 1. **Fix SQL injection vulnerabilities** in browse.ts endpoints 2. **Implement constant-time comparison** for API key verification 3. **Sanitize logs** to prevent sensitive data exposure ### Short-term (Within 1 week) 4. **Strengthen password requirements** to at least 12 characters with complexity 5. **Implement proper input validation** for all user inputs 6. **Add account lockout** after failed login attempts 7. **Audit and reduce console logging** of sensitive information ### Medium-term (Within 1 month) 8. **Implement log encryption** and automatic rotation 9. **Move master key** to environment variable or key management system 10. **Add comprehensive audit logging** for administrative actions 11. **Implement security headers** (CSP, X-Frame-Options, etc.) 12. **Add input validation middleware** for all API endpoints ### Long-term (Within 3 months) 13. **Security testing**: Implement automated security testing (SAST/DAST) 14. **Penetration testing**: Engage third-party security assessment 15. **Key rotation procedures**: Implement and document 16. **Security monitoring**: Add alerting for suspicious activity 17. **Compliance audit**: Formal compliance assessment if required --- ## Testing Recommendations ### Security Testing to Implement 1. **SQL Injection Testing**: Test all endpoints with SQLMap 2. **Authentication Testing**: Brute force, session management, token handling 3. **Authorization Testing**: Verify permission boundaries 4. **Input Validation**: Fuzz testing on all inputs 5. **API Security**: OWASP API Security Top 10 assessment 6. **Dependency Scanning**: Regular vulnerability scanning of npm packages ### Recommended Tools - **SAST**: Snyk, SonarQube, ESLint security plugins - **DAST**: OWASP ZAP, Burp Suite - **Dependency Scanning**: npm audit, Snyk, Dependabot - **Secrets Scanning**: GitGuardian, TruffleHog --- ## Conclusion The MySQL MCP WebUI has several critical security vulnerabilities that require immediate attention, particularly: 1. **SQL Injection vulnerabilities** in browse endpoints (CRITICAL) 2. **Timing attack vulnerability** in API key verification (CRITICAL) 3. **Sensitive data exposure** in logs (HIGH) The application does implement many security best practices, including encryption, password hashing, and permission controls. However, the identified issues could allow attackers to bypass these controls and gain unauthorized access to sensitive database information. **Immediate remediation of the CRITICAL issues is strongly recommended before deploying this application in a production environment with access to sensitive data.** --- ## Report Metadata - **Report Version**: 1.0 - **Audit Methodology**: Static Code Analysis - **Lines of Code Reviewed**: ~5,000+ lines - **Files Reviewed**: 30+ files - **Time Spent**: Comprehensive review - **Next Review**: Recommended after fixing critical issues --- **End of Security Audit Report**