# 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