cve-dashboard/docs/security/security-audit-2026-04-01.md

# Security Audit Report — STEAM Security Dashboard

**Date:** 2026-04-01
**Scope:** Full codebase — backend routes, authentication, file handling, Python scripts, React frontend
**Methodology:** Static analysis across four parallel audit tracks

---

## Executive Summary

The audit identified **31 findings** across four severity levels. The most serious issues are concentrated in the **authentication and authorization layer** — several endpoints are either completely unauthenticated or have role-checking middleware called with the wrong arguments, silently bypassing access control. These require immediate remediation before the application is exposed to a broader user base.

| Severity | Count |
|----------|-------|
| Critical | 6 |
| High | 9 |
| Medium | 10 |
| Low / Info | 6 |
| **Total** | **31** |

The application has strong foundational security in several areas: all database queries use parameterized statements (no SQL injection risk), path traversal prevention is comprehensive, Python script execution uses `spawn` with argument arrays (no shell injection), and file type allowlisting is in place. The vulnerabilities are largely in middleware wiring and missing access controls rather than fundamental design flaws.

---

## Critical Findings

---

### C-1 — Missing Authentication on Ivanti Findings Endpoints

**File:** `backend/routes/ivantiFindings.js:552–600`

The findings router imports `requireRole` but **not** `requireAuth`. No authentication middleware is applied at the router level or on individual routes. Four endpoints are fully unauthenticated:

```js
const { requireRole } = require('../middleware/auth');  // requireAuth never imported

router.get('/', async (req, res) => {          // line 552 — no auth
router.post('/sync', async (req, res) => {     // line 561 — no auth
router.get('/counts', async (req, res) => {    // line 571 — no auth
router.get('/fp-workflow-counts', ...)         // line 580 — no auth
```

**Impact:** Any unauthenticated attacker on the network can read the full list of Ivanti host findings (hostnames, IPs, CVEs, severity, SLA status), trigger a sync operation, and enumerate all finding metrics.

**Fix:** Import `requireAuth` and apply it to the router or each route:
```js
const { requireAuth, requireRole } = require('../middleware/auth');
router.use(requireAuth(db));
```

---

### C-2 — Broken requireRole Call — Privilege Escalation in Knowledge Base

**File:** `backend/routes/knowledgeBase.js:43, 305`

`requireRole` is called with `db` as the first argument:

```js
router.post('/upload', requireAuth(db), requireRole(db, 'editor', 'admin'), ...)
router.delete('/:id', requireAuth(db), requireRole(db, 'editor', 'admin'), ...)
```

The function signature is `function requireRole(...allowedRoles)`. It does not accept `db`. The database object is treated as the first "allowed role", so the check becomes `req.user.role === db` — an object comparison that always evaluates false, meaning **the check never blocks anyone**. Any authenticated viewer can upload and delete knowledge base documents.

**Fix:** Remove `db` from all `requireRole` calls:
```js
requireRole('editor', 'admin')
```

---

### C-3 — Unauthenticated Ivanti Finding Note Writes

**File:** `backend/routes/ivantiFindings.js:639`

The PUT endpoint for saving finding notes has no authentication middleware:

```js
router.put('/:findingId/note', (req, res) => {
    const note = String(req.body.note || '').slice(0, 255);
    db.run(`INSERT INTO ivanti_finding_notes ...`);
});
```

**Impact:** Any unauthenticated request can write notes to any finding. Notes are visible to all users and used during remediation triage. An attacker could inject false status information (e.g. "EXC-12345 — patched") to mislead the team or cover tracks.

**Fix:** Add `requireAuth(db)` to this route.

---

### C-4 — No Brute Force Protection on Login Endpoint

**File:** `backend/routes/auth.js:10`

The login endpoint has no rate limiting, attempt counting, or lockout:

```js
router.post('/login', async (req, res) => {
    const { username, password } = req.body;
    // Direct DB lookup, unlimited attempts
```

**Impact:** An attacker can run unlimited password guesses against any account at full network speed. With the default credentials documented in the README and displayed in the UI (see F-2), admin accounts are a trivial target.

**Fix:** Apply `express-rate-limit` to the login route:
```js
const rateLimit = require('express-rate-limit');
const loginLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 20 });
router.post('/login', loginLimiter, async (req, res) => { ... });
```

---

### C-5 — Default Credentials Displayed in Login UI

**File:** `frontend/src/components/LoginForm.js:104`

The login form renders hardcoded credentials in plain text:

```jsx
<p className="text-sm text-gray-500 text-center font-mono">
    Default: <span className="text-intel-accent">admin</span> /
             <span className="text-intel-accent">admin123</span>
</p>
```

**Impact:** Anyone who opens the login page — including unauthenticated users — sees the default admin credentials. Combined with C-4 (no rate limiting), this is a direct path to admin compromise if the password has not been changed.

**Fix:** Remove this block entirely. Document default credentials only in the deployment guide. Enforce password change on first login server-side.

---

### C-6 — Missing Sandbox Attribute on Knowledge Base PDF Iframe

**File:** `frontend/src/components/KnowledgeBaseViewer.js:195`

The inline document viewer renders uploaded files in an unsandboxed iframe:

```jsx
<iframe
    src={`${API_BASE}/knowledge-base/${article.id}/content`}
    title={article.title}
    className="w-full h-full rounded"
>
```

**Impact:** A malicious PDF or HTML file uploaded by an editor could execute JavaScript within the application's origin, accessing `localStorage`, `sessionStorage`, and DOM of the parent page. An attacker with editor access could upload a file that steals session data from any user who views it.

**Fix:** Add a restrictive `sandbox` attribute:
```jsx
<iframe
    sandbox="allow-same-origin allow-scripts"
    src={...}
    title={article.title}
/>
```

---

## High Findings

---

### H-1 — /cleanup-sessions Missing Role Check

**File:** `backend/routes/auth.js:223`

The comment says "admin only" but the endpoint only checks for any valid session:

```js
router.post('/cleanup-sessions', async (req, res) => {
    const sessionId = req.cookies?.session_id;
    if (!sessionId) return res.status(401).json({ error: '...' });
    // No role check
```

**Fix:** Apply `requireAuth(db)` and `requireRole('admin')`.

---

### H-2 — Hardcoded Fallback SESSION_SECRET

**File:** `backend/server.js:31`

```js
const SESSION_SECRET = process.env.SESSION_SECRET || 'default-secret-change-me';
```

If the `.env` file is missing or the variable is unset, all sessions are signed with a publicly known string. An attacker who knows the secret can forge valid session cookies.

**Fix:** Fail hard on startup if the secret is not set:
```js
const SESSION_SECRET = process.env.SESSION_SECRET;
if (!SESSION_SECRET) throw new Error('SESSION_SECRET environment variable must be set');
```

---

### H-3 — Audit Log Parameter Mismatch — Silent Audit Trail Gaps

**Files:** `backend/routes/archerTickets.js:89–95, 172, 206` and `backend/routes/knowledgeBase.js:235–244, 287–296`

The `logAudit` helper expects an object with `entityType` and `entityId`. These callers use the wrong keys (`targetType`, `targetId`) or pass positional arguments instead of an object:

```js
// archerTickets.js — wrong keys
logAudit(db, { ..., targetType: 'archer_ticket', targetId: this.lastID, ... });

// knowledgeBase.js — positional (wrong pattern)
logAudit(db, req.user.id, req.user.username, 'VIEW_KB_ARTICLE', 'knowledge_base', id, ...);
```

**Impact:** All Archer ticket and Knowledge Base operations produce audit log rows with `NULL` entity type and entity ID. Security investigations and compliance reviews will show these actions occurred but not what was affected.

**Fix:** Align all callers to the object format expected by `auditLog.js`:
```js
logAudit(db, { userId, username, action, entityType, entityId, details, ipAddress });
```

---

### H-4 — Viewers Can Write Compliance Notes

**Files:** `backend/routes/compliance.js:522` (also flagged by file-upload audit)

The POST /notes endpoint is protected by authentication but not by role:

```js
router.post('/notes', async (req, res) => {   // no requireRole()
```

**Impact:** Any viewer can add notes to any compliance item. Notes surface in the detail panel and influence remediation decisions. False notes cannot be deleted via the API.

**Fix:** `requireRole('editor', 'admin')` on this route.

---

### H-5 — Sync Endpoints Accessible to All Authenticated Users

**Files:** `backend/routes/ivantiFindings.js:561`, `backend/routes/ivantiWorkflows.js:262`

POST /sync on both routers requires only authentication, not editor/admin role. Any viewer can trigger expensive Ivanti API calls repeatedly.

**Impact:** Viewer-role users can cause repeated large API fetches, potentially hitting Ivanti rate limits and blocking legitimate syncs for the team.

**Fix:** Add `requireRole('editor', 'admin')` to both POST /sync routes.

---

### H-6 — HTTP Header Injection via Unsanitized Filename in Content-Disposition

**File:** `backend/routes/knowledgeBase.js:258, 299`

The original uploaded filename (user-controlled) is written directly into the `Content-Disposition` response header:

```js
res.setHeader('Content-Disposition', `inline; filename="${row.file_name}"`);
res.setHeader('Content-Disposition', `attachment; filename="${row.file_name}"`);
```

`row.file_name` stores `uploadedFile.originalname` which is not sanitized for use in HTTP headers. A filename containing `"\r\n` characters can split the response and inject arbitrary headers.

**Fix:**
```js
const safeFilename = row.file_name.replace(/["\r\n\\]/g, '');
res.setHeader('Content-Disposition', `attachment; filename="${safeFilename}"`);
```

---

### H-7 — Race Condition in Knowledge Base File Upload

**File:** `backend/routes/knowledgeBase.js:91–155`

The file is moved to its permanent location (line 93) before the database record is created (line 114). If the DB insert fails, the file is orphaned on disk. Two concurrent uploads with the same slug can also bypass the uniqueness check due to the async gap between the slug check query and the insert.

**Fix:** Keep the file in the temp directory until the DB insert succeeds, then move it:
```js
db.run(insertSql, [...], function(err) {
    if (err) { fs.unlinkSync(uploadedFile.path); return res.status(500)...; }
    fs.renameSync(uploadedFile.path, filePath);
    res.json({ success: true });
});
```

---

### H-8 — Hardcoded Default Admin Password in setup.js

**File:** `backend/setup.js:175`

```js
const passwordHash = await bcrypt.hash('admin123', 10);
```

If `setup.js` is re-run on an existing deployment (e.g. during a restore), the admin password resets to a known value. The password is also documented in the README and displayed in the login UI (C-5).

**Fix:** Generate a random password on first run and print it once to stdout, or require it as a CLI argument. Never hardcode credentials in source.

---

### H-9 — ReactMarkdown Renders HTML Without Sanitization

**File:** `frontend/src/components/KnowledgeBaseViewer.js:169–171`

```jsx
<ReactMarkdown>{content}</ReactMarkdown>
```

`ReactMarkdown` by default allows raw HTML in markdown (via `rehype-raw`). A knowledge base article containing `<img src=x onerror="...">` or `<script>` tags would execute JavaScript in the viewer's browser.

**Fix:** Add `rehype-sanitize`:
```jsx
import rehypeSanitize from 'rehype-sanitize';
<ReactMarkdown rehypePlugins={[rehypeSanitize]}>{content}</ReactMarkdown>
```

---

## Medium Findings

---

### M-1 — No CSRF Token Protection on State-Changing Requests

**Files:** All POST / PUT / DELETE routes

Cookies are `SameSite: lax` which provides partial protection, but `lax` still allows top-level cross-site navigations to carry cookies. No CSRF token is validated server-side. Combined with the permissive CORS configuration, cross-site request forgery is possible against editors and admins.

**Fix:** Either upgrade session cookie to `SameSite: strict`, or implement a CSRF token (double-submit cookie pattern or `csurf` middleware).

---

### M-2 — CORS Allows Credentials with Explicit Origin List

**File:** `backend/server.js:111–114`

```js
app.use(cors({ origin: CORS_ORIGINS, credentials: true }));
```

`credentials: true` with explicit origins means any subdomain compromise or DNS hijacking of a listed origin could allow cross-origin authenticated requests. This is the correct pattern for this use case, but worth hardening.

**Fix:** Ensure `CORS_ORIGINS` is reviewed whenever the deployment changes. Consider `SameSite: strict` on cookies to reduce reliance on CORS for CSRF protection.

---

### M-3 — No Rate Limiting on NVD API Proxy

**File:** `backend/routes/nvdLookup.js:13`

Any authenticated user can trigger NVD API calls in rapid succession. NVD enforces a 5 req/30s unauthenticated limit, which can be exhausted by a single user making 5 lookups.

**Fix:** Add a server-side 1-hour cache keyed by CVE ID to avoid repeated external lookups, plus a per-user rate limit.

---

### M-4 — Admin Self-Demotion Check Uses Loose Equality

**File:** `backend/routes/users.js:118`

```js
if (userId == req.user.id && role && role !== 'admin') {
```

Using `==` allows type coercion. If `userId` is passed as a different type than `req.user.id`, the comparison may not match correctly.

**Fix:** `String(userId) === String(req.user.id)`.

---

### M-5 — Missing Hostname Format Validation

**File:** `backend/routes/compliance.js:451`

The hostname route parameter is used in SQL queries and responses. Only length is checked (>300). No format validation rejects characters outside a valid hostname range.

**Fix:**
```js
if (!/^[a-zA-Z0-9._-]+$/.test(hostname)) {
    return res.status(400).json({ error: 'Invalid hostname format' });
}
```

---

### M-6 — Vendor Field Validated Before Trim

**File:** `backend/routes/ivantiTodoQueue.js:8, 56`

Vendor length is validated before `.trim()` is called. A string of 200 spaces passes validation but becomes an empty string after trimming, which then passes without a vendor value for FP/Archer items that require one.

**Fix:** Trim first, then validate length and presence.

---

### M-7 — Unsanitized Original Filename Stored in Compliance Temp JSON

**File:** `backend/routes/compliance.js:262`

```js
filename: req.file.originalname,   // user-controlled, unsanitized
```

The original filename is stored in the temp JSON and later echoed back to the frontend. Special characters could cause log injection or unexpected display issues.

**Fix:** `filename: sanitizePathSegment(req.file.originalname)`.

---

### M-8 — Hardcoded Frontend Origin in CSP Header

**File:** `backend/routes/knowledgeBase.js:261`

```js
res.setHeader('Content-Security-Policy',
    "frame-ancestors 'self' http://71.85.90.9:3000 http://localhost:3000");
```

IP address is hardcoded. If the deployment IP changes, the CSP header will block inline document viewing without an obvious error and require a code change.

**Fix:** Use `CORS_ORIGINS` from the environment variable.

---

### M-9 — Sensitive API Error Messages Forwarded to UI

**Files:** `frontend/src/App.js:801, 816, 847, 886`

```js
} catch (err) {
    alert(`Error: ${err.message}`);
}
```

Raw API error messages are displayed in browser alerts. If the backend leaks stack traces or query information in error responses, this information reaches the user directly.

**Fix:** Show generic user-facing messages; log details to the console in development only.

---

### M-10 — User-Supplied Data in window.confirm Dialogs

**File:** `frontend/src/App.js:806, 891`

```js
if (!window.confirm(`Delete ticket ${ticket.ticket_key}?`)) return;
```

A ticket with a crafted `ticket_key` value (e.g. containing newlines or misleading text) could produce a deceptive confirmation dialog used to social-engineer users.

**Fix:** Use a React modal component with escaped, controlled text instead of `window.confirm`.

---

## Low / Info Findings

---

### L-1 — Silent ROLLBACK on Compliance Transaction Failure

**File:** `backend/routes/compliance.js:167`

```js
await dbRun(db, 'ROLLBACK').catch(() => {});
```

If the rollback itself fails, the error is swallowed entirely. A failed rollback leaves an open transaction that can cause subsequent operations to block.

**Fix:** Log rollback failures even if execution continues.

---

### L-2 — Fire-and-Forget Audit Logging

**File:** `backend/helpers/auditLog.js:9`

Audit log writes fail silently. If the database is under load or unavailable, audit records are dropped with no alert.

**Fix:** Log audit write failures to stderr so they surface in server logs.

---

### L-3 — Async Temp File Cleanup With No Error Handling

**File:** `backend/routes/compliance.js:239, 247, 266, 281, 322`

```js
fs.unlink(req.file.path, () => {});
```

Cleanup failures accumulate silently, potentially causing disk exhaustion over time.

**Fix:** Log errors on unlink failure (excluding ENOENT which is expected).

---

### L-4 — IVANTI_SKIP_TLS Disables Certificate Validation

**File:** `backend/routes/ivantiFindings.js:385`

`IVANTI_SKIP_TLS=true` disables TLS verification for all Ivanti API calls, enabling man-in-the-middle attacks against the sync. It is controlled purely by environment variable with no warning.

**Fix:** Log a prominent warning on startup when this flag is active, and ensure it is never set in production.

---

### L-5 — console.error in Production Frontend Code

**Files:** `frontend/src/contexts/AuthContext.js:26`, `KnowledgeBaseViewer.js:31, 56`

Full error objects are logged to the browser console in production builds. In a monitored environment, these could expose internal details to anyone with DevTools open.

**Fix:** Guard with `if (process.env.NODE_ENV === 'development')` or use a structured logging library.

---

### L-6 — localStorage Column Config Lacks Structural Validation

**File:** `frontend/src/components/pages/ReportingPage.js:51–68`

Column order/visibility is loaded from `localStorage` and merged with defaults. If the stored data is tampered with (via XSS or DevTools), the parsed structure is used with only partial validation.

**Fix:** Validate each loaded item against the known `COLUMN_DEFS` whitelist before use (a `hasOwnProperty` check is already present; ensure it runs on every item before the merge).

---

## Summary Table

| ID | Severity | Title | File |
|----|----------|-------|------|
| C-1 | Critical | Missing auth on Ivanti findings endpoints | ivantiFindings.js:552 |
| C-2 | Critical | requireRole(db) call bypasses role check in KB routes | knowledgeBase.js:43,305 |
| C-3 | Critical | Unauthenticated finding note writes | ivantiFindings.js:639 |
| C-4 | Critical | No brute force protection on login | auth.js:10 |
| C-5 | Critical | Default credentials displayed in login UI | LoginForm.js:104 |
| C-6 | Critical | Missing sandbox on PDF/document iframe | KnowledgeBaseViewer.js:195 |
| H-1 | High | /cleanup-sessions missing role check | auth.js:223 |
| H-2 | High | Hardcoded fallback SESSION_SECRET | server.js:31 |
| H-3 | High | Audit log parameter mismatch — silent trail gaps | archerTickets.js, knowledgeBase.js |
| H-4 | High | Viewers can write compliance notes | compliance.js:522 |
| H-5 | High | Sync endpoints accessible to all authenticated users | ivantiFindings.js:561, ivantiWorkflows.js:262 |
| H-6 | High | HTTP header injection via Content-Disposition filename | knowledgeBase.js:258,299 |
| H-7 | High | Race condition in KB file upload | knowledgeBase.js:91 |
| H-8 | High | Hardcoded default admin password in setup.js | setup.js:175 |
| H-9 | High | ReactMarkdown renders HTML without sanitization | KnowledgeBaseViewer.js:169 |
| M-1 | Medium | No CSRF token protection | All state-changing routes |
| M-2 | Medium | CORS credentials with explicit origin list | server.js:111 |
| M-3 | Medium | No rate limiting on NVD API proxy | nvdLookup.js:13 |
| M-4 | Medium | Admin self-demotion check uses loose equality | users.js:118 |
| M-5 | Medium | Missing hostname format validation | compliance.js:451 |
| M-6 | Medium | Vendor field validated before trim | ivantiTodoQueue.js:8,56 |
| M-7 | Medium | Unsanitized original filename in temp JSON | compliance.js:262 |
| M-8 | Medium | Hardcoded frontend IP in CSP header | knowledgeBase.js:261 |
| M-9 | Medium | API error messages forwarded to UI | App.js:801,816,847,886 |
| M-10 | Medium | User data in window.confirm dialogs | App.js:806,891 |
| L-1 | Low | Silent ROLLBACK on transaction failure | compliance.js:167 |
| L-2 | Low | Fire-and-forget audit logging | auditLog.js:9 |
| L-3 | Low | Async temp file cleanup with no error handling | compliance.js:239+ |
| L-4 | Low | IVANTI_SKIP_TLS with no startup warning | ivantiFindings.js:385 |
| L-5 | Low | console.error exposed in production frontend | AuthContext.js, KnowledgeBaseViewer.js |
| L-6 | Low | localStorage column config lacks structural validation | ReportingPage.js:51 |

---

## Remediation Priority

### Immediate — fix before adding users

1. **C-1** — Add `requireAuth` import and router-level middleware to `ivantiFindings.js`
2. **C-2** — Remove `db` from all `requireRole(db, ...)` calls in `knowledgeBase.js`
3. **C-3** — Add `requireAuth(db)` to the finding note PUT route
4. **C-4** — Add `express-rate-limit` to the login route (20 attempts / 15 min)
5. **C-5** — Remove default credentials from `LoginForm.js`
6. **H-2** — Hard-fail on startup if `SESSION_SECRET` is not set in env

### Short-term — next maintenance window

7. **C-6** — Add `sandbox` attribute to the KB iframe
8. **H-3** — Fix `logAudit` call signatures in `archerTickets.js` and `knowledgeBase.js`
9. **H-4** — Add `requireRole('editor', 'admin')` to POST /compliance/notes
10. **H-5** — Add `requireRole('editor', 'admin')` to both POST /sync routes
11. **H-6** — Sanitize filename for `Content-Disposition` header
12. **H-7** — Move file after DB insert succeeds in KB upload
13. **H-8** — Remove hardcoded password from `setup.js`; generate random on first run
14. **H-9** — Add `rehype-sanitize` to `ReactMarkdown` usage

### Medium-term

15. **M-1** — Implement CSRF token or upgrade cookie to `SameSite: strict`
16. **M-3** — Add server-side CVE lookup cache
17. **M-5** — Add hostname format regex validation
18. **M-8** — Pull frontend origin from `CORS_ORIGINS` env var for CSP header
19. **M-9** — Replace `alert(err.message)` with user-friendly error messages
20. Remaining medium and low findings

---

## Positive Security Observations

The following were explicitly verified as secure and should be preserved:

- **SQL injection prevention** — all queries use SQLite3 parameterized statements throughout
- **Path traversal prevention** — `sanitizePathSegment()` and `isPathWithinUploads()` are comprehensive and consistently applied
- **Python script execution** — `spawn('python3', [SCRIPT, filePath])` passes arguments as an array, not a shell string — no command injection possible
- **Python scripts** — no `eval()`, `exec()`, `pickle.load()`, or shell calls in any script
- **File size enforcement** — 10 MB limit applied via multer before route handlers execute
- **File type allowlisting** — extension + MIME prefix validation applied at upload
- **Static file serving** — `express.static` with `{ dotfiles: 'deny', index: false }` prevents directory listing
- **Temp file path validation** — `isSafeTempPath()` enforces `.json` extension on compliance temp files
- **Password hashing** — bcrypt with cost factor 10 used throughout

---

*Audit scope: static analysis only. Dynamic testing (active exploitation, fuzzing, dependency CVE scan) not performed.*