diff --git a/security-issues.md b/security-issues.md new file mode 100644 index 0000000..1974d64 --- /dev/null +++ b/security-issues.md @@ -0,0 +1,636 @@ +# Security Issues - Code Review Report + +## 🔴 Critical Security Issues + +### 1. No Input Sanitization - XSS Vulnerability +**Severity:** CRITICAL (OWASP Top 10 #3) +**Files Affected:** +- `server/routes.ts` (all POST/PATCH endpoints) +- Frontend components rendering user content + +**Problem:** +User-supplied content stored and displayed without sanitization: +```typescript +// Backend - No sanitization +app.post('/api/articles', async (req, res) => { + const article = insertArticleSchema.parse(req.body); + await storage.createArticle({ + ...article, + content: req.body.content, // ❌ UNSANITIZED HTML/JavaScript! + title: req.body.title // ❌ UNSANITIZED! + }); +}); + +// Frontend - Dangerous rendering +
+``` + +**Attack Vector:** +```javascript +// Attacker posts article with malicious content +{ + title: "Normal Article", + content: "" +} +// When other users view this article, the script executes! +``` + +**Impact:** +- **Cookie theft** (session hijacking) +- **Credential theft** (keylogging) +- **Malware distribution** +- **Defacement** +- **Phishing attacks** +- Affects ALL users viewing malicious content + +**Affected Data:** +- Article content & titles +- Comments +- Media outlet descriptions +- User profile data +- Any user-generated content + +**Fix Solution:** +```typescript +// Backend sanitization +import DOMPurify from 'isomorphic-dompurify'; + +app.post('/api/articles', async (req, res) => { + const article = insertArticleSchema.parse(req.body); + + // ✅ Sanitize all user input + await storage.createArticle({ + ...article, + title: DOMPurify.sanitize(req.body.title, { + ALLOWED_TAGS: [] // Strip all HTML from titles + }), + content: DOMPurify.sanitize(req.body.content, { + ALLOWED_TAGS: ['p', 'br', 'b', 'i', 'u', 'a', 'img'], + ALLOWED_ATTR: ['href', 'src', 'alt'], + ALLOW_DATA_ATTR: false + }), + excerpt: DOMPurify.sanitize(req.body.excerpt, { + ALLOWED_TAGS: [] + }) + }); +}); + +// Frontend - Use safe rendering +import DOMPurify from 'dompurify'; + +// ✅ Sanitize before rendering +
+ +// OR better - use React components instead of HTML +
{article.content}
// React auto-escapes +``` + +**Priority:** P0 - IMMEDIATE FIX REQUIRED + +--- + +### 2. SQL Injection Risk (Potential) +**Severity:** CRITICAL (OWASP Top 10 #1) +**Files Affected:** `server/storage.ts` + +**Problem:** +While Drizzle ORM prevents most SQL injection, raw SQL queries are vulnerable: +```typescript +// ❌ If any raw SQL used with user input +const results = await db.execute( + sql`SELECT * FROM articles WHERE slug = ${userInput}` // Potentially unsafe +); +``` + +**Impact:** +- Database breach +- Data theft +- Data deletion +- Privilege escalation + +**Fix Solution:** +```typescript +// ✅ Always use parameterized queries +const results = await db + .select() + .from(articles) + .where(eq(articles.slug, userInput)); // Drizzle handles escaping + +// ✅ If raw SQL needed, use prepared statements +const stmt = db.prepare( + sql`SELECT * FROM articles WHERE slug = $1` +); +const results = await stmt.execute([userInput]); +``` + +**Priority:** P0 - Audit all queries + +--- + +### 3. Missing CSRF Protection +**Severity:** HIGH (OWASP Top 10 #8) +**Files Affected:** +- `server/index.ts` +- All state-changing endpoints + +**Problem:** +No CSRF tokens on state-changing operations: +```typescript +// ❌ POST/PUT/DELETE endpoints have no CSRF protection +app.post('/api/articles', async (req, res) => { + // Attacker can make user create article from malicious site! +}); + +app.delete('/api/articles/:id', async (req, res) => { + // Attacker can delete user's articles! +}); +``` + +**Attack Vector:** +```html + +
+ + +
+ + +``` + +**Impact:** +- Unauthorized actions +- Data manipulation +- Account takeover +- Financial loss (in bidding system) + +**Fix Solution:** +```typescript +// Install CSRF protection +import csrf from 'csurf'; + +const csrfProtection = csrf({ + cookie: { + httpOnly: true, + secure: process.env.NODE_ENV === 'production', + sameSite: 'strict' + } +}); + +// Apply to all state-changing routes +app.post('/api/*', csrfProtection); +app.put('/api/*', csrfProtection); +app.patch('/api/*', csrfProtection); +app.delete('/api/*', csrfProtection); + +// Send token to client +app.get('/api/csrf-token', csrfProtection, (req, res) => { + res.json({ csrfToken: req.csrfToken() }); +}); + +// Client must include token +fetch('/api/articles', { + method: 'POST', + headers: { + 'CSRF-Token': csrfToken, + 'Content-Type': 'application/json' + }, + body: JSON.stringify(data) +}); +``` + +**Priority:** P1 - Add before production + +--- + +### 4. Insufficient Authentication Checks +**Severity:** HIGH +**Files Affected:** `server/routes.ts` + +**Problem:** +Critical endpoints missing authentication: +```typescript +// ❌ Anyone can create articles! +app.post('/api/articles', async (req, res) => { + // No authentication check! +}); + +// ❌ Anyone can see all bids! +app.get('/api/bids', async (req, res) => { + // No authentication check! +}); + +// ❌ Anyone can modify media outlets! +app.patch('/api/media-outlets/:slug', async (req, res) => { + // Checks inside route, not as middleware +}); +``` + +**Impact:** +- Unauthorized data access +- Spam creation +- Data manipulation +- Privilege escalation + +**Fix Solution:** +```typescript +// ✅ Require authentication by default +const publicRoutes = [ + '/api/media-outlets', + '/api/articles', + '/api/auth/user' +]; + +app.use('/api/*', (req, res, next) => { + // Allow public routes + if (publicRoutes.some(route => req.path.startsWith(route))) { + return next(); + } + + // Require auth for everything else + return isAuthenticated(req, res, next); +}); + +// ✅ Explicit role checks +const requireAdmin = (req: any, res: Response, next: NextFunction) => { + if (!req.user || (req.user.role !== 'admin' && req.user.role !== 'superadmin')) { + return res.status(403).json({ message: "Admin access required" }); + } + next(); +}; + +app.post('/api/articles', isAuthenticated, requireAdmin, async (req, res) => { + // Only authenticated admins can create +}); +``` + +**Priority:** P1 - Fix before production + +--- + +### 5. Weak Session Configuration +**Severity:** HIGH +**Files Affected:** +- `server/simpleAuth.ts` +- `server/replitAuth.ts` + +**Problem:** +Session cookies lack security flags: +```typescript +// ❌ Insecure session config +app.use(session({ + secret: process.env.SESSION_SECRET || 'dev-secret', // ❌ Weak default + resave: false, + saveUninitialized: false, + cookie: { + // ❌ Missing security flags! + maxAge: 30 * 24 * 60 * 60 * 1000 // 30 days - too long! + } +})); +``` + +**Impact:** +- Session hijacking +- Cookie theft +- XSS escalation +- Man-in-the-middle attacks + +**Fix Solution:** +```typescript +// ✅ Secure session configuration +app.use(session({ + secret: process.env.SESSION_SECRET, // ✅ Require in production + resave: false, + saveUninitialized: false, + name: 'sessionId', // ✅ Don't reveal framework + cookie: { + httpOnly: true, // ✅ Prevent JavaScript access + secure: process.env.NODE_ENV === 'production', // ✅ HTTPS only in prod + sameSite: 'strict', // ✅ CSRF protection + maxAge: 24 * 60 * 60 * 1000, // ✅ 1 day (not 30!) + domain: process.env.COOKIE_DOMAIN // ✅ Limit to your domain + }, + store: sessionStore // ✅ Use persistent store (not memory) +})); + +// ✅ Validate SESSION_SECRET exists in production +if (process.env.NODE_ENV === 'production' && !process.env.SESSION_SECRET) { + throw new Error('SESSION_SECRET must be set in production'); +} +``` + +**Priority:** P1 - Fix immediately + +--- + +## 🟠 High Priority Security Issues + +### 6. No Rate Limiting - DoS Vulnerability +**Severity:** HIGH +**Files Affected:** All API endpoints + +**Problem:** +No rate limiting allows abuse: +```typescript +// ❌ Unlimited requests possible +app.post('/api/auctions/:id/bid', async (req, res) => { + // Attacker can spam bids! +}); + +app.post('/api/comments', async (req, res) => { + // Attacker can spam comments! +}); +``` + +**Impact:** +- Denial of Service +- Resource exhaustion +- Bid manipulation +- Spam flooding + +**Fix Solution:** +```typescript +import rateLimit from 'express-rate-limit'; + +// ✅ General API rate limit +const apiLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, // 15 minutes + max: 100, // 100 requests per window + message: 'Too many requests, please try again later', + standardHeaders: true, + legacyHeaders: false, +}); + +app.use('/api/', apiLimiter); + +// ✅ Strict limits for sensitive operations +const bidLimiter = rateLimit({ + windowMs: 60 * 1000, // 1 minute + max: 5, // 5 bids per minute + skipSuccessfulRequests: false +}); + +const commentLimiter = rateLimit({ + windowMs: 60 * 1000, + max: 10 // 10 comments per minute +}); + +app.post('/api/auctions/:id/bid', bidLimiter, async (req, res) => {}); +app.post('/api/comments', commentLimiter, async (req, res) => {}); + +// ✅ Login rate limiting (prevent brute force) +const loginLimiter = rateLimit({ + windowMs: 15 * 60 * 1000, + max: 5, // 5 login attempts per 15 minutes + skipSuccessfulRequests: true +}); + +app.post('/api/login', loginLimiter, async (req, res) => {}); +``` + +**Priority:** P1 - Add before production + +--- + +### 7. Sensitive Data Exposure in Logs +**Severity:** MEDIUM +**Files Affected:** Multiple routes + +**Problem:** +Logging sensitive user data: +```typescript +catch (error) { + console.error("Error:", error); + console.error("Request body:", req.body); // ❌ May contain passwords! +} +``` + +**Impact:** +- Password exposure +- Session token leaks +- PII in logs + +**Fix Solution:** +```typescript +// ✅ Sanitize logs +const sanitizeLog = (data: any) => { + const sensitive = ['password', 'token', 'secret', 'apiKey']; + const sanitized = { ...data }; + + for (const key of sensitive) { + if (key in sanitized) { + sanitized[key] = '[REDACTED]'; + } + } + + return sanitized; +}; + +catch (error) { + console.error("Error:", error.message); + console.error("Request body:", sanitizeLog(req.body)); +} +``` + +**Priority:** P2 - Implement logging standards + +--- + +### 8. Missing Security Headers +**Severity:** MEDIUM +**Files Affected:** `server/index.ts` + +**Problem:** +No security headers configured: +```typescript +// ❌ Missing security headers +app.use(express.json()); +// No helmet, no CSP, no security headers! +``` + +**Impact:** +- XSS attacks easier +- Clickjacking possible +- MIME sniffing attacks +- Missing defense-in-depth + +**Fix Solution:** +```typescript +import helmet from 'helmet'; + +// ✅ Add security headers +app.use(helmet({ + contentSecurityPolicy: { + directives: { + defaultSrc: ["'self'"], + styleSrc: ["'self'", "'unsafe-inline'"], + scriptSrc: ["'self'"], + imgSrc: ["'self'", "data:", "https:"], + connectSrc: ["'self'"], + fontSrc: ["'self'"], + objectSrc: ["'none'"], + mediaSrc: ["'self'"], + frameSrc: ["'none'"], + }, + }, + hsts: { + maxAge: 31536000, + includeSubDomains: true, + preload: true + }, + referrerPolicy: { policy: 'strict-origin-when-cross-origin' } +})); + +// ✅ Additional headers +app.use((req, res, next) => { + res.setHeader('X-Content-Type-Options', 'nosniff'); + res.setHeader('X-Frame-Options', 'DENY'); + res.setHeader('X-XSS-Protection', '1; mode=block'); + next(); +}); +``` + +**Priority:** P2 - Add security layer + +--- + +## 🟡 Medium Priority Security Issues + +### 9. No Content Length Limits +**Severity:** MEDIUM +**Files Affected:** `server/index.ts` + +**Problem:** +No request size limits: +```typescript +app.use(express.json()); // ❌ No size limit! +``` + +**Impact:** +- Memory exhaustion +- DoS via large payloads + +**Fix Solution:** +```typescript +// ✅ Limit request size +app.use(express.json({ limit: '10mb' })); // Reasonable limit +app.use(express.urlencoded({ limit: '10mb', extended: true })); +``` + +**Priority:** P2 - Prevent abuse + +--- + +### 10. Insecure Direct Object References (IDOR) +**Severity:** MEDIUM +**Files Affected:** Various endpoints + +**Problem:** +No ownership validation: +```typescript +// ❌ User can delete ANY comment by guessing ID! +app.delete('/api/comments/:id', async (req, res) => { + await storage.deleteComment(req.params.id); + // No check if user owns this comment! +}); +``` + +**Impact:** +- Unauthorized data access +- Data deletion +- Privilege escalation + +**Fix Solution:** +```typescript +// ✅ Validate ownership +app.delete('/api/comments/:id', isAuthenticated, async (req: any, res) => { + const comment = await storage.getCommentById(req.params.id); + + if (!comment) { + return res.status(404).json({ message: "Comment not found" }); + } + + // Check ownership or admin + if (comment.userId !== req.user.id && req.user.role !== 'admin') { + return res.status(403).json({ message: "Not authorized" }); + } + + await storage.deleteComment(req.params.id); + res.json({ success: true }); +}); +``` + +**Priority:** P2 - Add authorization checks + +--- + +## Security Checklist + +### Immediate Actions (P0-P1) +- [ ] Sanitize ALL user input (XSS prevention) +- [ ] Audit SQL queries for injection risks +- [ ] Implement CSRF protection +- [ ] Add authentication to all endpoints +- [ ] Secure session configuration +- [ ] Add rate limiting +- [ ] Review and fix privilege escalation risks + +### Before Production (P1-P2) +- [ ] Add security headers (Helmet) +- [ ] Implement proper logging (no sensitive data) +- [ ] Add request size limits +- [ ] Fix IDOR vulnerabilities +- [ ] Set up security monitoring +- [ ] Conduct security audit +- [ ] Penetration testing + +### Best Practices +- [ ] Regular security updates +- [ ] Dependency vulnerability scanning +- [ ] Security training for team +- [ ] Incident response plan +- [ ] Regular backups +- [ ] Security code review process + +## Summary + +| Severity | Count | Must Fix Before Production | +|----------|-------|----------------------------| +| 🔴 Critical | 5 | ✅ Yes - BLOCKING | +| 🟠 High | 3 | ✅ Yes | +| 🟡 Medium | 2 | ⚠️ Strongly Recommended | + +**Total Security Issues:** 10 + +**OWASP Top 10 Coverage:** +- ✅ A01: Broken Access Control (Issues #3, #4, #10) +- ✅ A02: Cryptographic Failures (Issue #5) +- ✅ A03: Injection (Issues #1, #2) +- ✅ A05: Security Misconfiguration (Issues #5, #8) +- ✅ A07: Identification/Authentication Failures (Issues #4, #5) +- ✅ A08: Software/Data Integrity Failures (Issue #3) + +**Estimated Fix Time:** +- Critical issues: 8-12 hours +- High priority: 4-6 hours +- Medium priority: 4-6 hours +- **Total: 16-24 hours** + +**Security Hardening Priority:** +1. Input sanitization (XSS) +2. CSRF protection +3. Authentication/authorization +4. Session security +5. Rate limiting +6. Security headers +7. Audit logging +8. IDOR fixes + +**Compliance Notes:** +- GDPR: Add data encryption, privacy controls +- PCI DSS: Required if handling payments +- SOC 2: Implement audit logging +- HIPAA: Not applicable unless handling health data