Address critical security vulnerabilities in the platform

Identifies and proposes solutions for XSS, SQL Injection risks, and missing CSRF protection across backend and frontend code. Replit-Commit-Author: Agent Replit-Commit-Session-Id: aabe2db1-f078-4501-aab5-be145ebc6b9a Replit-Commit-Checkpoint-Type: intermediate_checkpoint Replit-Commit-Screenshot-Url: https://storage.googleapis.com/screenshot-production-us-central1/3df548ff-50ae-432f-9be4-25d34eccc983/aabe2db1-f078-4501-aab5-be145ebc6b9a/TqVS1Ec
2025-10-12 08:29:28 +00:00
parent 49dfcba2b0
commit e89c8d7fee
1 changed files with 636 additions and 0 deletions
--- a/security-issues.md
+++ 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
+<div dangerouslySetInnerHTML={{ __html: article.content }} />
+```
+
+**Attack Vector:**
+```javascript
+// Attacker posts article with malicious content
+{
+  title: "Normal Article",
+  content: "<script>steal_cookies()</script><img src=x onerror='alert(document.cookie)'>"
+}
+// 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
+<div 
+  dangerouslySetInnerHTML={{ 
+    __html: DOMPurify.sanitize(article.content) 
+  }} 
+/>
+
+// OR better - use React components instead of HTML
+<div>{article.content}</div> // 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
+<!-- Attacker's website -->
+<form action="https://yourapp.com/api/articles" method="POST">
+  <input name="title" value="Spam Article">
+  <input name="content" value="Malicious content">
+</form>
+<script>document.forms[0].submit();</script>
+<!-- When logged-in user visits, article created in their name! -->
+```
+
+**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