Files
cve-dashboard/docs/security/security-audit-tracker.md

338 lines
16 KiB
Markdown

# Security Audit Tracker — STEAM Security Dashboard
**Last scan:** 2026-04-20
**Scope:** Full repository — backend routes, middleware, helpers, scripts, frontend components
**Baseline:** `docs/security-audit-2026-04-01.md` (31 findings), `docs/security-remediation-plan.md` (17 prioritised items)
---
## Table of Contents
- [Remediation Status — April 1 Audit](#remediation-status--april-1-audit)
- [New Findings — April 20 Scan](#new-findings--april-20-scan)
- [Open Finding Summary](#open-finding-summary)
- [Positive Security Observations](#positive-security-observations)
- [Scan Metadata](#scan-metadata)
---
## Remediation Status — April 1 Audit
Cross-reference of the 31 original findings against the current codebase. Status: **Fixed**, **Partial**, or **Open**.
### Critical Findings
| ID | Title | Status | Evidence |
|---|---|---|---|
| C-1 | Missing auth on Ivanti findings endpoints | **Fixed** | `ivantiFindings.js` — router uses `requireAuth(db)` at router level, `requireGroup` on sync |
| C-2 | `requireRole(db)` bypasses role check in KB routes | **Fixed** | `knowledgeBase.js` — uses `requireGroup('Admin', 'Standard_User')` correctly |
| C-3 | Unauthenticated finding note writes | **Fixed** | `ivantiFindings.js` — note routes behind `requireAuth(db)` |
| C-4 | No brute force protection on login | **Fixed** | `auth.js``loginLimiter` (20 attempts / 15 min) applied to POST /login |
| C-5 | Default credentials displayed in login UI | **Fixed** | `LoginForm.js` — no hardcoded credentials in the component |
| C-6 | Missing sandbox on KB document iframe | **Fixed** | `KnowledgeBaseViewer.js:282``sandbox="allow-same-origin"` applied |
### High Findings
| ID | Title | Status | Evidence |
|---|---|---|---|
| H-1 | `/cleanup-sessions` missing role check | **Fixed** | `auth.js``requireAuth(db), requireGroup('Admin')` applied |
| H-2 | Hardcoded fallback SESSION_SECRET | **Fixed** | `server.js:34-37` — hard-fails with `process.exit(1)` if unset |
| H-3 | Audit log parameter mismatch — silent trail gaps | **Partial** | `knowledgeBase.js` — fixed. `archerTickets.js``logAudit` calls missing `username` field (see N-1 below) |
| H-4 | Viewers can write compliance notes | **Fixed** | `compliance.js``requireGroup('Admin', 'Standard_User')` on POST /notes |
| H-5 | Sync endpoints accessible to all authenticated users | **Fixed** | Both `ivantiFindings.js` and `ivantiWorkflows.js``requireGroup('Admin', 'Standard_User')` on POST /sync |
| H-6 | HTTP header injection via Content-Disposition filename | **Fixed** | `knowledgeBase.js` — filename sanitized with `.replace(/["\r\n\\]/g, '')` |
| H-7 | Race condition in KB file upload | **Fixed** | `knowledgeBase.js` — file moved after DB insert succeeds |
| H-8 | Hardcoded default admin password in setup.js | **Fixed** | `setup.js` — generates random password via `crypto.randomBytes(12)` |
| H-9 | ReactMarkdown renders HTML without sanitization | **Fixed** | `KnowledgeBaseViewer.js``rehypeSanitize` plugin applied |
### Medium Findings
| ID | Title | Status | Evidence |
|---|---|---|---|
| M-1 | No CSRF token protection | **Open** | Cookies use `SameSite: lax` — no CSRF token implemented |
| M-2 | CORS credentials with explicit origin list | **Open** | Acceptable for this deployment model — monitor |
| M-3 | No rate limiting on NVD API proxy | **Open** | No server-side cache or per-user rate limit on `/api/nvd/lookup` |
| M-4 | Admin self-demotion check uses loose equality | **Fixed** | `users.js` — uses `String(userId) === String(req.user.id)` |
| M-5 | Missing hostname format validation | **Fixed** | `compliance.js` POST /notes — regex validation `^[a-zA-Z0-9._-]+$` |
| M-6 | Vendor field validated before trim | **Open** | `ivantiTodoQueue.js:8``isValidVendor()` checks length before trim |
| M-7 | Unsanitized original filename in temp JSON | **Open** | `compliance.js:344``req.file.originalname` passed directly |
| M-8 | Hardcoded frontend IP in CSP header | **Fixed** | `knowledgeBase.js:302` — reads from `CORS_ORIGINS` env var |
| M-9 | API error messages forwarded to UI | **Open** | Frontend still uses `alert(err.message)` in several places |
| M-10 | User data in window.confirm dialogs | **Open** | Frontend still uses `window.confirm` with user-supplied data |
### Low / Info Findings
| ID | Title | Status | Evidence |
|---|---|---|---|
| L-1 | Silent ROLLBACK on transaction failure | **Open** | `compliance.js:167``.catch(() => {})` still swallows errors |
| L-2 | Fire-and-forget audit logging | **Partial** | `auditLog.js` — now logs to `console.error` on failure, but no alerting |
| L-3 | Async temp file cleanup with no error handling | **Open** | `compliance.js``fs.unlink(path, () => {})` still used |
| L-4 | IVANTI_SKIP_TLS with no startup warning | **Open** | No startup warning when `IVANTI_SKIP_TLS=true` |
| L-5 | console.error in production frontend | **Open** | No environment guard on console.error calls |
| L-6 | localStorage column config lacks structural validation | **Open** | No change observed |
### Remediation Plan Items (not in original 31)
| ID | Title | Status | Evidence |
|---|---|---|---|
| RP-1 | Authenticate /uploads static file access | **Open** | `server.js:127``express.static('uploads')` still unauthenticated |
| RP-2 | Sanitize Mermaid SVG output with DOMPurify | **Open** | `KnowledgeBaseViewer.js:38``innerHTML = svg` without DOMPurify |
| RP-3 | Strip server file paths from compliance preview response | **Open** | `compliance.js:342` — full `tempFilePath` returned to client |
| RP-4 | Add SESSION_SECRET to .env.example | **Open** | `.env.example` — no `SESSION_SECRET` entry |
---
## New Findings — April 20 Scan
Findings discovered in this scan that were not present in the April 1 audit.
---
### N-1 — Archer Ticket Audit Logs Missing `username` Field (Medium)
**File:** `backend/routes/archerTickets.js:89, 172, 195`
All three `logAudit` calls in the Archer tickets router omit the `username` field:
```js
logAudit(db, {
userId: req.user.id,
action: 'CREATE_ARCHER_TICKET',
// username: req.user.username ← missing
...
});
```
The `auditLog.js` helper defaults missing username to `'unknown'`, so all Archer ticket audit entries show `username = 'unknown'` instead of the actual user.
**Impact:** Audit trail for Archer ticket operations cannot identify which user performed the action. Compliance reviews and incident investigations are degraded.
**Fix:** Add `username: req.user.username` to all three `logAudit` calls.
---
### N-2 — `migrate-to-1.1.js` Contains Hardcoded Admin Password (Medium)
**File:** `backend/migrate-to-1.1.js:246`
```js
const passwordHash = await bcrypt.hash('admin123', 10);
```
While `setup.js` was fixed to generate random passwords (H-8), the migration script still hardcodes `admin123`. If this migration is run on an existing deployment, it resets the admin password to a known value.
**Impact:** Running the migration on a production system resets the admin account to a publicly known password.
**Fix:** Either generate a random password (matching `setup.js` pattern) or skip admin creation if the user already exists.
---
### N-3 — Compliance Preview Returns Full Server Filesystem Path (Medium)
**File:** `backend/routes/compliance.js:342`
```js
tempFile: tempFilePath,
```
The preview endpoint returns the full server-side path (e.g. `/home/cve-dashboard/backend/uploads/temp/compliance_preview_...json`) to the frontend. The commit endpoint then receives this path back and reads the file. This exposes the server's directory structure to any authenticated user.
**Impact:** Information disclosure — authenticated users learn the server's absolute filesystem layout, which aids further exploitation.
**Fix:** Return only the filename. Reconstruct the full path server-side in the commit handler:
```js
tempFile: tempFilename, // just the basename
// In commit handler:
const tempFile = path.join(TEMP_DIR, path.basename(req.body.tempFile));
```
---
### N-4 — `/uploads` Static Directory Served Without Authentication (High)
**File:** `backend/server.js:127`
```js
app.use('/uploads', express.static('uploads', {
dotfiles: 'deny',
index: false
}));
```
All uploaded files (CVE documents, compliance data, knowledge base articles) are served as static files without any authentication check. Anyone who knows or guesses a file URL can access sensitive vulnerability documentation, compliance reports, and internal knowledge base content.
**Impact:** Unauthenticated access to all uploaded documents. File paths are predictable (CVE ID + vendor + timestamp-filename pattern).
**Fix:** Replace with an authenticated route handler:
```js
app.use('/uploads', requireAuth(db), express.static('uploads', { ... }));
```
---
### N-5 — Mermaid SVG Rendered via `innerHTML` Without Sanitization (Medium)
**File:** `frontend/src/components/KnowledgeBaseViewer.js:38`
```js
ref.current.innerHTML = svg;
```
Mermaid-generated SVG is injected directly into the DOM via `innerHTML`. While Mermaid itself sanitizes most input, a crafted diagram definition in a knowledge base article could potentially produce SVG with embedded event handlers or script elements.
**Impact:** Stored XSS vector if Mermaid's internal sanitization is bypassed. Any user viewing the article would execute the payload.
**Fix:** Sanitize the SVG string before injection:
```js
import DOMPurify from 'dompurify';
ref.current.innerHTML = DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } });
```
---
### N-6 — `SESSION_SECRET` Not Documented in `.env.example` (Low)
**File:** `backend/.env.example`
The `SESSION_SECRET` environment variable is required for the server to start (hard-fail added per H-2 fix), but it is not listed in `.env.example`. Fresh deployments will fail with no guidance on what to set.
**Fix:** Add to `.env.example`:
```
# Session signing secret — generate with: openssl rand -hex 32
SESSION_SECRET=
```
---
### N-7 — `requireGroup` Error Response Leaks Current User Group (Low)
**File:** `backend/middleware/auth.js:55-60`
```js
return res.status(403).json({
error: 'Insufficient permissions',
required: allowedGroups,
current: req.user.group
});
```
The 403 response includes both the required groups and the user's current group. This is minor information disclosure — an attacker probing endpoints learns the exact group membership of the compromised account and which groups are needed.
**Fix:** Remove `required` and `current` from the response:
```js
return res.status(403).json({ error: 'Insufficient permissions' });
```
---
### N-8 — No Content-Security-Policy Header on Main Application (Medium)
**File:** `backend/server.js:107-113`
Security headers include `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Protection`, `Referrer-Policy`, and `Permissions-Policy`, but no `Content-Security-Policy` header. CSP is the primary browser-side defense against XSS.
**Impact:** No browser-enforced restriction on script sources. If an XSS vulnerability exists (e.g. N-5), there is no CSP to mitigate it.
**Fix:** Add a baseline CSP header:
```js
res.setHeader('Content-Security-Policy',
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; " +
"img-src 'self' data:; font-src 'self'; connect-src 'self'");
```
Start with `Content-Security-Policy-Report-Only` to avoid breaking existing functionality.
---
### N-9 — Expired Sessions Not Cleaned Up Automatically (Low)
**File:** `backend/server.js`, `backend/routes/auth.js`
The `sessions` table has no automatic cleanup. Expired sessions accumulate indefinitely. The `/cleanup-sessions` endpoint exists but must be triggered manually by an admin.
**Impact:** Performance degradation over time as the sessions table grows. Not directly exploitable, but expired session rows increase the surface for timing attacks on session lookups.
**Fix:** Add a cleanup interval on server startup:
```js
setInterval(() => {
db.run("DELETE FROM sessions WHERE expires_at < datetime('now')");
}, 6 * 60 * 60 * 1000); // every 6 hours
```
---
## Open Finding Summary
Prioritised list of all open findings requiring action.
### High Priority
| ID | Severity | Title | Source |
|---|---|---|---|
| N-4 | High | `/uploads` static directory served without authentication | New |
### Medium Priority
| ID | Severity | Title | Source |
|---|---|---|---|
| M-1 | Medium | No CSRF token protection | April 1 |
| M-3 | Medium | No rate limiting on NVD API proxy | April 1 |
| N-1 | Medium | Archer ticket audit logs missing `username` field | New |
| N-2 | Medium | `migrate-to-1.1.js` contains hardcoded admin password | New |
| N-3 | Medium | Compliance preview returns full server filesystem path | New |
| N-5 | Medium | Mermaid SVG rendered via `innerHTML` without sanitization | New |
| N-8 | Medium | No Content-Security-Policy header on main application | New |
| M-6 | Medium | Vendor field validated before trim | April 1 |
| M-7 | Medium | Unsanitized original filename in temp JSON | April 1 |
| M-9 | Medium | API error messages forwarded to UI | April 1 |
| M-10 | Medium | User data in `window.confirm` dialogs | April 1 |
### Low Priority
| ID | Severity | Title | Source |
|---|---|---|---|
| N-6 | Low | `SESSION_SECRET` not documented in `.env.example` | New |
| N-7 | Low | `requireGroup` error response leaks current user group | New |
| N-9 | Low | Expired sessions not cleaned up automatically | New |
| L-1 | Low | Silent ROLLBACK on transaction failure | April 1 |
| L-3 | Low | Async temp file cleanup with no error handling | April 1 |
| L-4 | Low | IVANTI_SKIP_TLS with no startup warning | April 1 |
| L-5 | Low | console.error in production frontend | April 1 |
| L-6 | Low | localStorage column config lacks structural validation | April 1 |
---
## Positive Security Observations
Verified secure patterns that should be preserved:
- **SQL injection prevention** — all queries use parameterized statements throughout the entire codebase
- **Path traversal prevention** — `sanitizePathSegment()` and `isPathWithinUploads()` consistently applied in `server.js`, `compliance.js`, and `knowledgeBase.js`
- **Python script execution** — `spawn('python3', [SCRIPT, filePath])` with argument arrays — no shell injection
- **File upload security** — extension allowlist + MIME prefix validation + 10 MB size limit via multer
- **Password hashing** — bcrypt with cost factor 10 used for all password storage
- **Session management** — 32-byte random session IDs via `crypto.randomBytes`, httpOnly cookies, 24h expiry
- **Rate limiting** — login endpoint protected with 20 attempts per 15-minute window
- **Audit trail** — comprehensive audit logging on all state-changing operations (with noted exceptions above)
- **Self-modification prevention** — admin cannot demote or deactivate their own account
- **Ownership-scoped deletion** — Standard_User can only delete resources they created
- **Compliance linkage protection** — deletion blocked when tickets are linked to active compliance reports
- **Temp file path validation** — `isSafeTempPath()` enforces `.json` extension and `uploads/temp/` directory
- **Static file serving** — `dotfiles: 'deny'` and `index: false` prevent directory listing
---
## Scan Metadata
| Field | Value |
|---|---|
| Scan date | 2026-04-20 |
| Scan type | Full repository static analysis |
| Scope | `backend/`, `frontend/src/`, config files |
| Baseline | `docs/security-audit-2026-04-01.md` |
| Previous findings | 31 (6 Critical, 9 High, 10 Medium, 6 Low/Info) |
| Remediated | 20 fully fixed, 2 partially fixed |
| Still open (from baseline) | 13 |
| New findings | 9 |
| Total open | 22 (1 High, 11 Medium, 10 Low) |
| Methodology | Static analysis — code review of all route handlers, middleware, helpers, and frontend components |