mirror of
https://github.com/Dvorinka/MyClubServer.git
synced 2026-06-04 02:32:57 +00:00
upload
This commit is contained in:
@@ -0,0 +1,422 @@
|
||||
# Security Audit Report
|
||||
**Date:** October 15, 2025
|
||||
**Auditor:** Cascade AI
|
||||
**Scope:** Full application security review
|
||||
|
||||
---
|
||||
|
||||
## Executive Summary
|
||||
|
||||
This comprehensive security audit examined authentication, authorization, input validation, data protection, and API security across the fotbal-club application. The application demonstrates **good security practices** in most areas, but several **critical and high-priority vulnerabilities** require immediate attention.
|
||||
|
||||
**Overall Security Rating:** ⚠️ **MEDIUM-HIGH RISK**
|
||||
|
||||
---
|
||||
|
||||
## 🔴 Critical Findings
|
||||
|
||||
### 1. **Exposed API Keys and Credentials in .env File**
|
||||
**Severity:** CRITICAL
|
||||
**Location:** `.env` (lines 84, 96)
|
||||
|
||||
**Issue:**
|
||||
- Real OpenRouter API key exposed: `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`
|
||||
- Real Umami Analytics credentials exposed: `admin / eevRQ6h3G@!c#y4A1T`
|
||||
|
||||
**Impact:**
|
||||
- Unauthorized API usage and potential billing charges
|
||||
- Complete compromise of analytics account
|
||||
- These credentials are committed to version control
|
||||
|
||||
**Recommendation:**
|
||||
```bash
|
||||
# IMMEDIATE ACTION REQUIRED:
|
||||
# 1. Rotate all exposed credentials NOW
|
||||
# 2. Remove from .env and use .env.example for templates
|
||||
# 3. Add .env to .gitignore
|
||||
# 4. Review git history and consider repository secrets scanning
|
||||
# 5. Use environment-specific secrets management (Azure Key Vault, AWS Secrets Manager, etc.)
|
||||
```
|
||||
|
||||
### 2. **Weak Default JWT Secret**
|
||||
**Severity:** CRITICAL
|
||||
**Location:** `pkg/utils/jwt.go` (line 67), `.env` (line 20)
|
||||
|
||||
**Issue:**
|
||||
```go
|
||||
func GetJWTSecret() string {
|
||||
secret := os.Getenv("JWT_SECRET")
|
||||
if secret == "" {
|
||||
return "default-secret-key-change-in-production" // ⚠️ Weak default
|
||||
}
|
||||
return secret
|
||||
}
|
||||
```
|
||||
|
||||
Current `.env` also uses weak secret: `your_jwt_secret_key_here`
|
||||
|
||||
**Impact:**
|
||||
- JWT tokens can be forged if default is used in production
|
||||
- Complete authentication bypass possible
|
||||
- All user sessions compromised
|
||||
|
||||
**Recommendation:**
|
||||
```go
|
||||
// Fail hard if JWT secret is missing or weak
|
||||
func GetJWTSecret() string {
|
||||
secret := os.Getenv("JWT_SECRET")
|
||||
if secret == "" || secret == "default-secret-key-change-in-production" ||
|
||||
secret == "your_jwt_secret_key_here" || len(secret) < 32 {
|
||||
log.Fatal("SECURITY: JWT_SECRET must be set to a strong secret (minimum 32 characters)")
|
||||
}
|
||||
return secret
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** `main.go` (line 184) checks this for production only - should check ALL environments.
|
||||
|
||||
---
|
||||
|
||||
## 🟠 High-Priority Findings
|
||||
|
||||
### 3. **Dev Bypass Middleware Allows Admin Access Without Authentication**
|
||||
**Severity:** HIGH
|
||||
**Location:** `internal/middleware/auth.go` (lines 78-92)
|
||||
|
||||
**Issue:**
|
||||
```go
|
||||
func DevBypass() gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
if config.AppConfig != nil && config.AppConfig.AppEnv != "production" {
|
||||
if strings.ToLower(c.GetHeader("X-Dev-Admin")) == "true" {
|
||||
c.Set("userRole", "admin")
|
||||
c.Set("user", &models.User{Role: "admin"}) // ⚠️ Fake admin user
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
}
|
||||
c.Next()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- Any request with `X-Dev-Admin: true` header gets admin access in non-production
|
||||
- No authentication required
|
||||
- Development/staging environments fully compromised
|
||||
|
||||
**Recommendation:**
|
||||
```go
|
||||
// Remove DevBypass entirely or restrict to localhost only
|
||||
func DevBypass() gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
// Only allow from localhost
|
||||
if config.AppConfig != nil && config.AppConfig.AppEnv == "development" {
|
||||
if c.ClientIP() == "127.0.0.1" || c.ClientIP() == "::1" {
|
||||
if strings.ToLower(c.GetHeader("X-Dev-Admin")) == "true" {
|
||||
log.Println("WARNING: DevBypass used from localhost")
|
||||
c.Set("userRole", "admin")
|
||||
c.Set("user", &models.User{Role: "admin"})
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
c.Next()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### 4. **Admin Access Token Bypass**
|
||||
**Severity:** HIGH
|
||||
**Location:** `internal/middleware/auth.go` (lines 18-27)
|
||||
|
||||
**Issue:**
|
||||
```go
|
||||
if config.AppConfig != nil && config.AppConfig.AdminAccessToken != "" {
|
||||
header := c.GetHeader("X-Admin-Token")
|
||||
if header != "" && header == config.AppConfig.AdminAccessToken {
|
||||
c.Set("userRole", "admin")
|
||||
c.Set("user", &models.User{Role: "admin"})
|
||||
c.Next()
|
||||
return
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Impact:**
|
||||
- Bypasses normal JWT authentication
|
||||
- Single token compromise = full admin access
|
||||
- No rate limiting on this check
|
||||
- Token doesn't expire
|
||||
|
||||
**Recommendation:**
|
||||
- Remove this feature entirely OR
|
||||
- Implement proper token rotation and expiration
|
||||
- Add rate limiting
|
||||
- Log all usage
|
||||
- Restrict to specific IP ranges
|
||||
|
||||
### 5. **Insufficient Input Sanitization**
|
||||
**Severity:** HIGH
|
||||
**Location:** `pkg/utils/sanitize.go`
|
||||
|
||||
**Issue:**
|
||||
- Custom regex-based HTML sanitization is prone to bypasses
|
||||
- No use of industry-standard sanitization library (bluemonday)
|
||||
- Comment on line 9 acknowledges: "For production, consider using bluemonday library"
|
||||
|
||||
**Impact:**
|
||||
- XSS vulnerabilities possible
|
||||
- HTML injection attacks
|
||||
- JavaScript execution in user-controlled content
|
||||
|
||||
**Recommendation:**
|
||||
```bash
|
||||
# Install bluemonday
|
||||
go get github.com/microcosm-cc/bluemonday
|
||||
|
||||
# Replace regex-based sanitization with bluemonday
|
||||
```
|
||||
|
||||
```go
|
||||
import "github.com/microcosm-cc/bluemonday"
|
||||
|
||||
func SanitizeHTML(html string) string {
|
||||
p := bluemonday.UGCPolicy() // User-generated content policy
|
||||
return p.Sanitize(html)
|
||||
}
|
||||
```
|
||||
|
||||
### 6. **Frontend XSS Risk - dangerouslySetInnerHTML Usage**
|
||||
**Severity:** HIGH
|
||||
**Location:** Multiple frontend files
|
||||
|
||||
**Affected Files:**
|
||||
- `frontend/src/pages/AboutPage.tsx` (4 occurrences)
|
||||
- `frontend/src/pages/ArticleDetailPage.tsx` (1 occurrence)
|
||||
- `frontend/src/pages/ActivityDetailPage.tsx` (1 occurrence - using DOMPurify ✓)
|
||||
- `frontend/src/pages/ClubPage.tsx` (1 occurrence)
|
||||
- `frontend/src/pages/admin/NewsletterAdminPage.tsx` (2 occurrences - using sanitizeHtml ✓)
|
||||
|
||||
**Issue:**
|
||||
Some uses sanitize properly, others don't consistently:
|
||||
|
||||
```tsx
|
||||
// ⚠️ Direct use without sanitization
|
||||
<Box dangerouslySetInnerHTML={{ __html: settings.about_html as any }} />
|
||||
|
||||
// ✅ Proper sanitization
|
||||
<Box dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(String(data.description)) }} />
|
||||
```
|
||||
|
||||
**Recommendation:**
|
||||
- Ensure ALL `dangerouslySetInnerHTML` uses are sanitized with DOMPurify
|
||||
- Create a wrapper component to enforce sanitization
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Medium-Priority Findings
|
||||
|
||||
### 7. **CSRF Protection Gaps**
|
||||
**Severity:** MEDIUM
|
||||
**Location:** `internal/middleware/csrf.go`
|
||||
|
||||
**Issues:**
|
||||
1. **In-memory token storage** (line 19): Tokens lost on server restart
|
||||
2. **No CSRF on Bearer token requests** (line 62): Skips CSRF if Bearer header present
|
||||
3. **1-hour token expiry**: May be too short for long forms
|
||||
|
||||
**Recommendation:**
|
||||
```go
|
||||
// Use Redis or database for token persistence
|
||||
// Consider requiring CSRF for sensitive operations even with Bearer tokens
|
||||
// Extend token lifetime to 4-8 hours for better UX
|
||||
```
|
||||
|
||||
### 8. **Rate Limiting Not Applied to All Endpoints**
|
||||
**Severity:** MEDIUM
|
||||
**Location:** `internal/routes/routes.go`
|
||||
|
||||
**Issue:**
|
||||
Rate limiting only on specific endpoints:
|
||||
- Login: 15/minute
|
||||
- Register: 5/hour
|
||||
- File upload: 30/minute
|
||||
- Contact form: 10/minute
|
||||
- Newsletter: 30/minute
|
||||
- Poll voting: 10/minute
|
||||
|
||||
Missing rate limiting on:
|
||||
- Password reset verification
|
||||
- Admin API endpoints
|
||||
- File operations (delete, scan)
|
||||
- Newsletter subscriber management
|
||||
|
||||
**Recommendation:**
|
||||
Apply global rate limiting middleware with per-endpoint overrides.
|
||||
|
||||
### 9. **SQL Injection - Low Risk (GORM Protected)**
|
||||
**Severity:** LOW (Informational)
|
||||
**Location:** Throughout controllers
|
||||
|
||||
**Finding:**
|
||||
Application uses GORM ORM properly with parameterized queries. No raw SQL detected in controllers. Examples:
|
||||
|
||||
```go
|
||||
// ✅ Safe - parameterized
|
||||
db.Where("email = ?", email).First(&user)
|
||||
db.Where("LOWER(email) = LOWER(?)", email).First(&user)
|
||||
```
|
||||
|
||||
**Status:** ✅ **PASS** - No SQL injection vulnerabilities detected
|
||||
|
||||
### 10. **Password Policy**
|
||||
**Severity:** MEDIUM
|
||||
**Location:** `internal/controllers/auth_controller.go` (line 21)
|
||||
|
||||
**Issue:**
|
||||
```go
|
||||
Password string `json:"password" binding:"required,min=8"`
|
||||
```
|
||||
|
||||
Minimum 8 characters, but no complexity requirements.
|
||||
|
||||
**Recommendation:**
|
||||
```go
|
||||
// Add password validation
|
||||
func ValidatePassword(password string) error {
|
||||
if len(password) < 12 {
|
||||
return errors.New("password must be at least 12 characters")
|
||||
}
|
||||
// Check for uppercase, lowercase, number, special char
|
||||
hasUpper := regexp.MustCompile(`[A-Z]`).MatchString(password)
|
||||
hasLower := regexp.MustCompile(`[a-z]`).MatchString(password)
|
||||
hasNumber := regexp.MustCompile(`[0-9]`).MatchString(password)
|
||||
hasSpecial := regexp.MustCompile(`[!@#$%^&*(),.?":{}|<>]`).MatchString(password)
|
||||
|
||||
if !(hasUpper && hasLower && hasNumber && hasSpecial) {
|
||||
return errors.New("password must contain uppercase, lowercase, number, and special character")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
### 11. **File Upload Security**
|
||||
**Severity:** MEDIUM
|
||||
**Location:** `internal/config/config.go`, `internal/controllers/files_controller.go`
|
||||
|
||||
**Findings:**
|
||||
✅ **Good practices:**
|
||||
- File size limits (10MB)
|
||||
- MIME type whitelist
|
||||
- Filename sanitization (`pkg/utils/sanitize.go`)
|
||||
|
||||
⚠️ **Issues:**
|
||||
- No virus scanning
|
||||
- SVG files allowed (can contain scripts)
|
||||
- File content not validated (MIME type spoofing possible)
|
||||
|
||||
**Recommendation:**
|
||||
```go
|
||||
// 1. Remove SVG from allowed types or sanitize thoroughly
|
||||
// 2. Validate file content, not just extension
|
||||
// 3. Store uploads outside webroot
|
||||
// 4. Consider ClamAV integration for virus scanning
|
||||
```
|
||||
|
||||
### 12. **Session Management**
|
||||
**Severity:** MEDIUM
|
||||
**Location:** JWT implementation
|
||||
|
||||
**Issues:**
|
||||
- JWT tokens are stateless - no revocation mechanism
|
||||
- 24-hour expiration (reasonable but fixed)
|
||||
- No refresh token mechanism
|
||||
- HttpOnly cookies used (✅ good) but no rotation
|
||||
|
||||
**Recommendation:**
|
||||
Implement token blacklist or use short-lived access tokens with refresh tokens.
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Security Strengths
|
||||
|
||||
The application demonstrates several **excellent security practices**:
|
||||
|
||||
1. ✅ **Password Hashing:** Uses bcrypt with proper cost (`pkg/utils/password.go`)
|
||||
2. ✅ **HttpOnly Cookies:** Prevents XSS token theft (`internal/controllers/auth_controller.go` line 150-158)
|
||||
3. ✅ **Secure Cookie Flags:** Sets Secure flag when using HTTPS
|
||||
4. ✅ **CORS Configuration:** Properly configured with origin validation (`main.go` line 111-130)
|
||||
5. ✅ **Security Headers:** X-Content-Type-Options, X-Frame-Options, HSTS (`main.go` line 99-105)
|
||||
6. ✅ **Content Security Policy:** Configurable CSP header
|
||||
7. ✅ **Admin Password Verification:** Required when modifying admin accounts (`internal/controllers/auth_controller.go` line 491)
|
||||
8. ✅ **Email Case-Insensitive Lookup:** Prevents duplicate accounts with different casing
|
||||
9. ✅ **Graceful Shutdown:** Proper cleanup on server shutdown (`main.go`)
|
||||
10. ✅ **Database Connection Pooling:** Configured limits prevent resource exhaustion
|
||||
|
||||
---
|
||||
|
||||
## Priority Action Items
|
||||
|
||||
### Immediate (Within 24 hours):
|
||||
1. **Rotate all exposed credentials** in `.env` file
|
||||
2. **Remove .env from version control**, use .env.example
|
||||
3. **Set strong JWT_SECRET** (minimum 32 characters, random)
|
||||
4. **Disable or restrict DevBypass middleware**
|
||||
|
||||
### Short-term (Within 1 week):
|
||||
5. **Replace custom HTML sanitization** with bluemonday
|
||||
6. **Audit all dangerouslySetInnerHTML** usage in frontend
|
||||
7. **Remove or secure Admin Access Token** feature
|
||||
8. **Implement comprehensive rate limiting**
|
||||
|
||||
### Medium-term (Within 1 month):
|
||||
9. **Add password complexity requirements**
|
||||
10. **Implement JWT token blacklist** or refresh tokens
|
||||
11. **Add file content validation** for uploads
|
||||
12. **Implement persistent CSRF token storage**
|
||||
|
||||
---
|
||||
|
||||
## Compliance Notes
|
||||
|
||||
### GDPR Considerations:
|
||||
- ✅ Newsletter unsubscribe mechanism present
|
||||
- ✅ Email preferences management
|
||||
- ⚠️ Consider adding data export/deletion endpoints
|
||||
|
||||
### OWASP Top 10 (2021):
|
||||
- **A01 Broken Access Control:** ⚠️ DevBypass and AdminAccessToken issues
|
||||
- **A02 Cryptographic Failures:** ⚠️ Weak default JWT secret
|
||||
- **A03 Injection:** ✅ GORM protects against SQL injection
|
||||
- **A04 Insecure Design:** ⚠️ In-memory CSRF tokens
|
||||
- **A05 Security Misconfiguration:** ⚠️ Exposed credentials
|
||||
- **A07 Identification/Authentication Failures:** ⚠️ Weak password policy
|
||||
- **A08 Software/Data Integrity:** ⚠️ No subresource integrity checks
|
||||
|
||||
---
|
||||
|
||||
## Testing Recommendations
|
||||
|
||||
1. **Penetration Testing:** Engage security professionals for thorough testing
|
||||
2. **Dependency Scanning:** Use `go mod` security scanning tools
|
||||
3. **SAST/DAST:** Implement automated security scanning in CI/CD
|
||||
4. **Secret Scanning:** Use tools like GitGuardian or Gitleaks
|
||||
|
||||
---
|
||||
|
||||
## Conclusion
|
||||
|
||||
The fotbal-club application has a **solid security foundation** with proper use of bcrypt, GORM ORM, security headers, and CORS. However, **critical vulnerabilities** exist around exposed credentials, weak JWT defaults, and authentication bypasses that must be addressed immediately.
|
||||
|
||||
**Recommended next steps:**
|
||||
1. Fix all CRITICAL issues within 24 hours
|
||||
2. Schedule HIGH priority fixes within 1 week
|
||||
3. Implement continuous security monitoring
|
||||
4. Conduct regular security audits
|
||||
|
||||
---
|
||||
|
||||
**Report End**
|
||||
For questions or clarifications, review the specific file locations and code references provided above.
|
||||
Reference in New Issue
Block a user