Security Audit - Multiple Vulnerabilities Identified

[MalteKiefer]

Summary

I conducted a security audit of the FastCP codebase and identified several vulnerabilities ranging from critical to low severity. This report outlines the findings with the goal of improving the overall security posture of the project.

---

Critical Severity (6 issues)

1. Command Injection in Password Verification

File: internal/auth/auth.go:157-173

The Python script for password verification uses string interpolation with insufficient escaping:

safePassword := strings.ReplaceAll(password, "'", "\\'")
cmd := exec.Command("python3", "-c", fmt.Sprintf(script, safeUsername, safePassword))

The escaping can be bypassed using sequences like \' which break out of the quoted context.

2. Command Injection in Password Fallback

File: internal/auth/auth.go:187-198

Similar issue in the bash fallback script for password verification.

3-4. Command Injection in SSH Key Management

File: internal/api/auth.go:412, 445

SSH public keys are embedded directly into shell commands:

cmd := exec.Command("bash", "-c", fmt.Sprintf("echo '%s' >> %s", keyLine, authKeysPath))

Attackers can inject commands via newlines or $(...) sequences in the public key.

5. Hardcoded Default Credentials

File: internal/config/config.go:91-94

Default credentials are hardcoded and exposed in the login page HTML:

• Admin password: fastcp2024!
• JWT Secret: change-this-secret-in-production-please

6. API Key Validation Bypass

File: internal/middleware/auth.go:87-91

API keys are only validated by checking the fcp_ prefix - no actual lookup against stored keys:

if !strings.HasPrefix(apiKey, "fcp_") {
    http.Error(w, `{"error": "invalid API key"}`, http.StatusUnauthorized)
    return
}

---

High Severity (5 issues)

Issue	File	Description
SQL Injection Risk	internal/database/manager.go:480-506	MySQL queries use fmt.Sprintf with backtick escaping for database/user names
Plaintext API Key Storage	internal/auth/auth.go:265-268	API keys stored in plaintext (code comment acknowledges this)
Dangerous CORS Configuration	internal/api/router.go:76	AllowedOrigins: ["*"] combined with AllowCredentials: true
Weak Random Generator	internal/sites/manager.go:1246-1252	WordPress salts use time.Now().UnixNano() instead of crypto/rand
Unsanitized Username in Commands	internal/api/users.go	Usernames from URL parameters used directly in system commands

---

Medium Severity (5 issues)

Issue	File	Description
Path Disclosure	internal/api/sites.go:229	Internal file paths exposed in error messages
Default Credentials in UI	internal/api/router.go:413	Login page displays default credentials
Token Storage	web/src/lib/api.ts:10	JWT stored in localStorage (XSS vulnerable)
No Rate Limiting	internal/api/auth.go:38-75	Login endpoint lacks rate limiting
Insufficient Impersonation Audit	internal/middleware/auth.go:52-64	Admin impersonation not fully logged

---

Low Severity (3 issues)

• Verbose error messages exposing internal details
• No HSTS header or HTTPS redirect enforcement
• Long JWT expiry (24h) without token rotation

---

Positive Security Aspects

I also want to highlight some good security practices already in place:

• Constant-time comparison for config-based authentication
• JWT algorithm validation preventing algorithm confusion attacks
• Domain validation with regex and shell metacharacter blacklist
• ZipSlip protection in WordPress extraction
• Path traversal checks in site manager
• Restricted file permissions (0600) for config files
• User isolation with sites restricted to /home/{user}/www/

---

Recommended Remediation Priority

1. Immediate: Fix command injection vulnerabilities in auth.go and api/auth.go
2. Immediate: Force credential/secret change on first installation
3. High: Implement actual API key validation
4. High: Restrict CORS configuration
5. Medium: Add rate limiting to authentication endpoints
6. Medium: Use parameterized queries for MySQL operations

---

Offer to Contribute

Would you like assistance in fixing these vulnerabilities? I would be happy to contribute pull requests addressing these issues if that would be helpful. Please let me know which issues you'd like me to prioritize, or if you prefer to handle them internally.

I'm also available to discuss any of these findings in more detail or provide additional context.

[rehmatworks]

@MalteKiefer Would you please find some time to audit it again? It is a new rewrite with several architectural improvements. Moving forward, this version will be maintained in the long run.

[rehmatworks]

I am closing this issue because it is a new rewrite, and I'd appreciate your help auditing the updated code. We will continue with a new issue if vulnerabilities or bugs are found.