|
|
|
@@ -1,6 +1,6 @@
|
|
|
|
# Security Audit Tracker — STEAM Security Dashboard
|
|
|
|
# Security Audit Tracker — STEAM Security Dashboard
|
|
|
|
|
|
|
|
|
|
|
|
**Last scan:** 2026-04-20
|
|
|
|
**Last scan:** 2026-06-04
|
|
|
|
**Scope:** Full repository — backend routes, middleware, helpers, scripts, frontend components
|
|
|
|
**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)
|
|
|
|
**Baseline:** `docs/security-audit-2026-04-01.md` (31 findings), `docs/security-remediation-plan.md` (17 prioritised items)
|
|
|
|
|
|
|
|
|
|
|
|
@@ -10,6 +10,7 @@
|
|
|
|
|
|
|
|
|
|
|
|
- [Remediation Status — April 1 Audit](#remediation-status--april-1-audit)
|
|
|
|
- [Remediation Status — April 1 Audit](#remediation-status--april-1-audit)
|
|
|
|
- [New Findings — April 20 Scan](#new-findings--april-20-scan)
|
|
|
|
- [New Findings — April 20 Scan](#new-findings--april-20-scan)
|
|
|
|
|
|
|
|
- [New Findings — June 4 Scan](#new-findings--june-4-scan)
|
|
|
|
- [Open Finding Summary](#open-finding-summary)
|
|
|
|
- [Open Finding Summary](#open-finding-summary)
|
|
|
|
- [Positive Security Observations](#positive-security-observations)
|
|
|
|
- [Positive Security Observations](#positive-security-observations)
|
|
|
|
- [Scan Metadata](#scan-metadata)
|
|
|
|
- [Scan Metadata](#scan-metadata)
|
|
|
|
@@ -54,37 +55,37 @@ Cross-reference of the 31 original findings against the current codebase. Status
|
|
|
|
| M-3 | No rate limiting on NVD API proxy | **Open** | No server-side cache or per-user rate limit on `/api/nvd/lookup` |
|
|
|
|
| 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-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-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-6 | Vendor field validated before trim | **Fixed** | `ivantiTodoQueue.js:11-14` — `isValidVendor()` now trims before length check |
|
|
|
|
| M-7 | Unsanitized original filename in temp JSON | **Open** | `compliance.js:344` — `req.file.originalname` passed directly |
|
|
|
|
| M-7 | Unsanitized original filename in temp JSON | **Open** | `compliance.js:347` — `req.file.originalname.replace(/[^\w.\-() ]/g, '_')` sanitizes in temp JSON, but line 355 returns raw `req.file.originalname` to client |
|
|
|
|
| M-8 | Hardcoded frontend IP in CSP header | **Fixed** | `knowledgeBase.js:302` — reads from `CORS_ORIGINS` env var |
|
|
|
|
| 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-9 | API error messages forwarded to UI | **Open** | Frontend still uses `alert(err.message)` in App.js, UserManagement.js, KnowledgeBasePage.js, JiraPage.js, and AdminPage.js |
|
|
|
|
| M-10 | User data in window.confirm dialogs | **Open** | Frontend still uses `window.confirm` with user-supplied data |
|
|
|
|
| M-10 | User data in window.confirm dialogs | **Partial** | App.js replaced with `ConfirmModal`. `JiraPage.js:430` still uses `window.confirm('Delete this Jira ticket record?')` — static string, reduced risk |
|
|
|
|
|
|
|
|
|
|
|
|
### Low / Info Findings
|
|
|
|
### Low / Info Findings
|
|
|
|
|
|
|
|
|
|
|
|
| ID | Title | Status | Evidence |
|
|
|
|
| ID | Title | Status | Evidence |
|
|
|
|
|---|---|---|---|
|
|
|
|
|---|---|---|---|
|
|
|
|
| L-1 | Silent ROLLBACK on transaction failure | **Open** | `compliance.js:167` — `.catch(() => {})` still swallows errors |
|
|
|
|
| L-1 | Silent ROLLBACK on transaction failure | **Fixed** | `compliance.js` — no `.catch(() => {})` patterns remain; ROLLBACK is followed by `throw err` |
|
|
|
|
| L-2 | Fire-and-forget audit logging | **Partial** | `auditLog.js` — now logs to `console.error` on failure, but no alerting |
|
|
|
|
| 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-3 | Async temp file cleanup with no error handling | **Open** | `compliance.js` — `fs.unlink(path, () => {})` still used in 5 locations |
|
|
|
|
| L-4 | IVANTI_SKIP_TLS with no startup warning | **Open** | No startup warning when `IVANTI_SKIP_TLS=true` |
|
|
|
|
| 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-5 | console.error in production frontend | **Open** | No environment guard on console.error calls — 30+ instances across frontend |
|
|
|
|
| L-6 | localStorage column config lacks structural validation | **Open** | No change observed |
|
|
|
|
| L-6 | localStorage column config lacks structural validation | **Open** | `ReportingPage.js:65-80` — parses JSON with try/catch but validates only `Array.isArray(saved)`, not element structure |
|
|
|
|
|
|
|
|
|
|
|
|
### Remediation Plan Items (not in original 31)
|
|
|
|
### Remediation Plan Items (not in original 31)
|
|
|
|
|
|
|
|
|
|
|
|
| ID | Title | Status | Evidence |
|
|
|
|
| ID | Title | Status | Evidence |
|
|
|
|
|---|---|---|---|
|
|
|
|
|---|---|---|---|
|
|
|
|
| RP-1 | Authenticate /uploads static file access | **Open** | `server.js:127` — `express.static('uploads')` still unauthenticated |
|
|
|
|
| RP-1 | Authenticate /uploads static file access | **Fixed** | `server.js:127` — `requireAuth()` middleware applied before `express.static('uploads')` |
|
|
|
|
| RP-2 | Sanitize Mermaid SVG output with DOMPurify | **Open** | `KnowledgeBaseViewer.js:38` — `innerHTML = svg` without DOMPurify |
|
|
|
|
| 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-3 | Strip server file paths from compliance preview response | **Fixed** | `compliance.js` and `vclMultiVertical.js` — preview returns only the temp filename; commit handler reconstructs full path via `path.join(TEMP_DIR, path.basename(tempFile))` |
|
|
|
|
| RP-4 | Add SESSION_SECRET to .env.example | **Open** | `.env.example` — no `SESSION_SECRET` entry |
|
|
|
|
| RP-4 | Add SESSION_SECRET to .env.example | **Fixed** | `.env.example` — `SESSION_SECRET=` with generation comment present |
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## New Findings — April 20 Scan
|
|
|
|
## New Findings — April 20 Scan
|
|
|
|
|
|
|
|
|
|
|
|
Findings discovered in this scan that were not present in the April 1 audit.
|
|
|
|
Findings discovered in the April 20 scan that were not present in the April 1 audit.
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
@@ -94,8 +95,8 @@ Findings discovered in this scan that were not present in the April 1 audit.
|
|
|
|
|
|
|
|
|
|
|
|
All three `logAudit` calls in the Archer tickets router omit the `username` field:
|
|
|
|
All three `logAudit` calls in the Archer tickets router omit the `username` field:
|
|
|
|
|
|
|
|
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
logAudit(db, {
|
|
|
|
logAudit({
|
|
|
|
userId: req.user.id,
|
|
|
|
userId: req.user.id,
|
|
|
|
action: 'CREATE_ARCHER_TICKET',
|
|
|
|
action: 'CREATE_ARCHER_TICKET',
|
|
|
|
// username: req.user.username ← missing
|
|
|
|
// username: req.user.username ← missing
|
|
|
|
@@ -113,60 +114,19 @@ The `auditLog.js` helper defaults missing username to `'unknown'`, so all Archer
|
|
|
|
|
|
|
|
|
|
|
|
### N-2 — `migrate-to-1.1.js` Contains Hardcoded Admin Password (Medium)
|
|
|
|
### N-2 — `migrate-to-1.1.js` Contains Hardcoded Admin Password (Medium)
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `backend/migrate-to-1.1.js:246`
|
|
|
|
**Status:** **Fixed** — file removed from codebase (confirmed 2026-06-04)
|
|
|
|
|
|
|
|
|
|
|
|
```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)
|
|
|
|
### N-3 — Compliance Preview Returns Full Server Filesystem Path (Medium) — FIXED
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `backend/routes/compliance.js:342`
|
|
|
|
**Status:** Fixed — preview now returns only the temp filename (`tempFilename`). The commit handler reconstructs the full path server-side via `path.join(TEMP_DIR, path.basename(tempFile))`. Applied to both `compliance.js` and `vclMultiVertical.js`.
|
|
|
|
|
|
|
|
|
|
|
|
```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)
|
|
|
|
### N-4 — `/uploads` Static Directory Served Without Authentication (High) — FIXED
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `backend/server.js:127`
|
|
|
|
**Status:** Fixed — `requireAuth()` middleware is now applied before `express.static('uploads')` in `server.js`. All file access now requires a valid session cookie.
|
|
|
|
|
|
|
|
|
|
|
|
```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', { ... }));
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
@@ -174,7 +134,7 @@ app.use('/uploads', requireAuth(db), express.static('uploads', { ... }));
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `frontend/src/components/KnowledgeBaseViewer.js:38`
|
|
|
|
**File:** `frontend/src/components/KnowledgeBaseViewer.js:38`
|
|
|
|
|
|
|
|
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
ref.current.innerHTML = svg;
|
|
|
|
ref.current.innerHTML = svg;
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
@@ -183,7 +143,7 @@ Mermaid-generated SVG is injected directly into the DOM via `innerHTML`. While M
|
|
|
|
**Impact:** Stored XSS vector if Mermaid's internal sanitization is bypassed. Any user viewing the article would execute the payload.
|
|
|
|
**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:
|
|
|
|
**Fix:** Sanitize the SVG string before injection:
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
import DOMPurify from 'dompurify';
|
|
|
|
import DOMPurify from 'dompurify';
|
|
|
|
ref.current.innerHTML = DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } });
|
|
|
|
ref.current.innerHTML = DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } });
|
|
|
|
```
|
|
|
|
```
|
|
|
|
@@ -192,15 +152,7 @@ ref.current.innerHTML = DOMPurify.sanitize(svg, { USE_PROFILES: { svg: true } })
|
|
|
|
|
|
|
|
|
|
|
|
### N-6 — `SESSION_SECRET` Not Documented in `.env.example` (Low)
|
|
|
|
### N-6 — `SESSION_SECRET` Not Documented in `.env.example` (Low)
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `backend/.env.example`
|
|
|
|
**Status:** **Fixed** — `.env.example` now includes `SESSION_SECRET=` with generation instructions (confirmed 2026-06-04)
|
|
|
|
|
|
|
|
|
|
|
|
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=
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
@@ -208,7 +160,7 @@ SESSION_SECRET=
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `backend/middleware/auth.js:55-60`
|
|
|
|
**File:** `backend/middleware/auth.js:55-60`
|
|
|
|
|
|
|
|
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
return res.status(403).json({
|
|
|
|
return res.status(403).json({
|
|
|
|
error: 'Insufficient permissions',
|
|
|
|
error: 'Insufficient permissions',
|
|
|
|
required: allowedGroups,
|
|
|
|
required: allowedGroups,
|
|
|
|
@@ -219,7 +171,7 @@ return res.status(403).json({
|
|
|
|
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.
|
|
|
|
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:
|
|
|
|
**Fix:** Remove `required` and `current` from the response:
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
return res.status(403).json({ error: 'Insufficient permissions' });
|
|
|
|
return res.status(403).json({ error: 'Insufficient permissions' });
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
@@ -234,7 +186,7 @@ Security headers include `X-Content-Type-Options`, `X-Frame-Options`, `X-XSS-Pro
|
|
|
|
**Impact:** No browser-enforced restriction on script sources. If an XSS vulnerability exists (e.g. N-5), there is no CSP to mitigate it.
|
|
|
|
**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:
|
|
|
|
**Fix:** Add a baseline CSP header:
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
res.setHeader('Content-Security-Policy',
|
|
|
|
res.setHeader('Content-Security-Policy',
|
|
|
|
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; " +
|
|
|
|
"default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; " +
|
|
|
|
"img-src 'self' data:; font-src 'self'; connect-src 'self'");
|
|
|
|
"img-src 'self' data:; font-src 'self'; connect-src 'self'");
|
|
|
|
@@ -252,14 +204,62 @@ The `sessions` table has no automatic cleanup. Expired sessions accumulate indef
|
|
|
|
**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.
|
|
|
|
**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:
|
|
|
|
**Fix:** Add a cleanup interval on server startup:
|
|
|
|
```js
|
|
|
|
```javascript
|
|
|
|
setInterval(() => {
|
|
|
|
setInterval(() => {
|
|
|
|
db.run("DELETE FROM sessions WHERE expires_at < datetime('now')");
|
|
|
|
pool.query("DELETE FROM sessions WHERE expires_at < NOW()");
|
|
|
|
}, 6 * 60 * 60 * 1000); // every 6 hours
|
|
|
|
}, 6 * 60 * 60 * 1000); // every 6 hours
|
|
|
|
```
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
## New Findings — June 4 Scan
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Findings discovered in the June 4, 2026 scan.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
### N-10 — Feedback Route Uses Incorrect TLS Option Name (Low)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `backend/routes/feedback.js:92, 221`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
```javascript
|
|
|
|
|
|
|
|
const reqOpts = {
|
|
|
|
|
|
|
|
// ...
|
|
|
|
|
|
|
|
rejectAuthorized: false,
|
|
|
|
|
|
|
|
};
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
The option is spelled `rejectAuthorized` instead of the correct `rejectUnauthorized`. This means the option is silently ignored by Node's `https` module — TLS verification remains **enabled** (the secure default), so this is not a vulnerability in itself. However, it indicates the developer intended to disable TLS verification (likely for Charter's SSL inspection proxy), and the code does not achieve its intent. If the GitLab instance uses an internal CA certificate, feedback submissions will fail with a TLS error when the proxy is active.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Impact:** Feedback submissions may fail behind the SSL inspection proxy due to unintended TLS verification. Not a security vulnerability — TLS remains enforced.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Fix:** If TLS skip is intentional, use the correct option name and gate it behind an env var:
|
|
|
|
|
|
|
|
```javascript
|
|
|
|
|
|
|
|
rejectUnauthorized: process.env.GITLAB_SKIP_TLS === 'true' ? false : true,
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
If TLS skip is not intentional, remove the incorrect option entirely.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
### N-11 — `window.confirm` Persists in JiraPage.js (Low)
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**File:** `frontend/src/components/pages/JiraPage.js:430`
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
```javascript
|
|
|
|
|
|
|
|
if (!window.confirm('Delete this Jira ticket record?')) return;
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
The main `App.js` was migrated to use `ConfirmModal` (a themed replacement for `window.confirm`), but `JiraPage.js` still uses the raw browser `confirm()` dialog. While the confirmation string is static (not user-supplied data), this is inconsistent with the security pattern established elsewhere in the codebase.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Impact:** Low — the string is static so there is no XSS vector. This is a consistency issue rather than a vulnerability.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
**Fix:** Replace with the `ConfirmModal` pattern used in `App.js`:
|
|
|
|
|
|
|
|
```javascript
|
|
|
|
|
|
|
|
setPendingConfirm({ message: 'Delete this Jira ticket record?', onConfirm: () => doDelete(id) });
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
## Open Finding Summary
|
|
|
|
## Open Finding Summary
|
|
|
|
|
|
|
|
|
|
|
|
Prioritised list of all open findings requiring action.
|
|
|
|
Prioritised list of all open findings requiring action.
|
|
|
|
@@ -268,7 +268,7 @@ Prioritised list of all open findings requiring action.
|
|
|
|
|
|
|
|
|
|
|
|
| ID | Severity | Title | Source |
|
|
|
|
| ID | Severity | Title | Source |
|
|
|
|
|---|---|---|---|
|
|
|
|
|---|---|---|---|
|
|
|
|
| N-4 | High | `/uploads` static directory served without authentication | New |
|
|
|
|
| — | — | (none — all High findings resolved) | — |
|
|
|
|
|
|
|
|
|
|
|
|
### Medium Priority
|
|
|
|
### Medium Priority
|
|
|
|
|
|
|
|
|
|
|
|
@@ -276,24 +276,21 @@ Prioritised list of all open findings requiring action.
|
|
|
|
|---|---|---|---|
|
|
|
|
|---|---|---|---|
|
|
|
|
| M-1 | Medium | No CSRF token protection | April 1 |
|
|
|
|
| M-1 | Medium | No CSRF token protection | April 1 |
|
|
|
|
| M-3 | Medium | No rate limiting on NVD API proxy | 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-1 | Medium | Archer ticket audit logs missing `username` field | April 20 |
|
|
|
|
| N-2 | Medium | `migrate-to-1.1.js` contains hardcoded admin password | New |
|
|
|
|
| N-5 / RP-2 | Medium | Mermaid SVG rendered via `innerHTML` without sanitization | April 20 |
|
|
|
|
| N-3 | Medium | Compliance preview returns full server filesystem path | New |
|
|
|
|
| N-8 | Medium | No Content-Security-Policy header on main application | April 20 |
|
|
|
|
| N-5 | Medium | Mermaid SVG rendered via `innerHTML` without sanitization | New |
|
|
|
|
| M-7 | Medium | Unsanitized original filename returned to client in preview response | April 1 |
|
|
|
|
| N-8 | Medium | No Content-Security-Policy header on main application | New |
|
|
|
|
| M-9 | Medium | API error messages forwarded to UI via `alert()` | April 1 |
|
|
|
|
| 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
|
|
|
|
### Low Priority
|
|
|
|
|
|
|
|
|
|
|
|
| ID | Severity | Title | Source |
|
|
|
|
| ID | Severity | Title | Source |
|
|
|
|
|---|---|---|---|
|
|
|
|
|---|---|---|---|
|
|
|
|
| N-6 | Low | `SESSION_SECRET` not documented in `.env.example` | New |
|
|
|
|
| M-10 | Low | User data in `window.confirm` dialogs (partially fixed — JiraPage only) | April 1 |
|
|
|
|
| N-7 | Low | `requireGroup` error response leaks current user group | New |
|
|
|
|
| N-7 | Low | `requireGroup` error response leaks current user group | April 20 |
|
|
|
|
| N-9 | Low | Expired sessions not cleaned up automatically | New |
|
|
|
|
| N-9 | Low | Expired sessions not cleaned up automatically | April 20 |
|
|
|
|
| L-1 | Low | Silent ROLLBACK on transaction failure | April 1 |
|
|
|
|
| N-10 | Low | Feedback route uses incorrect TLS option name | June 4 |
|
|
|
|
|
|
|
|
| N-11 | Low | `window.confirm` persists in JiraPage.js | June 4 |
|
|
|
|
| L-3 | Low | Async temp file cleanup with no error handling | 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-4 | Low | IVANTI_SKIP_TLS with no startup warning | April 1 |
|
|
|
|
| L-5 | Low | console.error in production frontend | April 1 |
|
|
|
|
| L-5 | Low | console.error in production frontend | April 1 |
|
|
|
|
@@ -305,19 +302,23 @@ Prioritised list of all open findings requiring action.
|
|
|
|
|
|
|
|
|
|
|
|
Verified secure patterns that should be preserved:
|
|
|
|
Verified secure patterns that should be preserved:
|
|
|
|
|
|
|
|
|
|
|
|
- **SQL injection prevention** — all queries use parameterized statements throughout the entire codebase
|
|
|
|
- **SQL injection prevention** — all queries use parameterized statements throughout the entire codebase (PostgreSQL `$1` placeholders)
|
|
|
|
- **Path traversal prevention** — `sanitizePathSegment()` and `isPathWithinUploads()` consistently applied in `server.js`, `compliance.js`, and `knowledgeBase.js`
|
|
|
|
- **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
|
|
|
|
- **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
|
|
|
|
- **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
|
|
|
|
- **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
|
|
|
|
- **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
|
|
|
|
- **Rate limiting** — login endpoint protected with 20 attempts per 15-minute window; password change limited to 5 per 15 minutes
|
|
|
|
- **Audit trail** — comprehensive audit logging on all state-changing operations (with noted exceptions above)
|
|
|
|
- **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
|
|
|
|
- **Self-modification prevention** — admin cannot demote or deactivate their own account
|
|
|
|
- **Ownership-scoped deletion** — Standard_User can only delete resources they created
|
|
|
|
- **Ownership-scoped deletion** — Standard_User can only delete resources they created
|
|
|
|
- **Compliance linkage protection** — deletion blocked when tickets are linked to active compliance reports
|
|
|
|
- **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
|
|
|
|
- **Temp file path validation** — `isSafeTempPath()` enforces `.json` extension and `uploads/temp/` directory
|
|
|
|
- **Static file serving** — `dotfiles: 'deny'` and `index: false` prevent directory listing
|
|
|
|
- **Static file serving** — `dotfiles: 'deny'` and `index: false` prevent directory listing
|
|
|
|
|
|
|
|
- **Webhook authentication** — GitLab webhook validates `x-gitlab-token` against `GITLAB_WEBHOOK_SECRET` env var
|
|
|
|
|
|
|
|
- **SESSION_SECRET enforcement** — server hard-fails on startup if `SESSION_SECRET` is not set
|
|
|
|
|
|
|
|
- **Input validation coverage** — CVE ID, vendor, hostname, metric_id, EXC number, and workflow_type all validated with regex or enum checks
|
|
|
|
|
|
|
|
- **Error response discipline** — backend routes consistently return `'Internal server error.'` for 500s, avoiding stack trace leaks
|
|
|
|
|
|
|
|
|
|
|
|
---
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
@@ -325,13 +326,16 @@ Verified secure patterns that should be preserved:
|
|
|
|
|
|
|
|
|
|
|
|
| Field | Value |
|
|
|
|
| Field | Value |
|
|
|
|
|---|---|
|
|
|
|
|---|---|
|
|
|
|
| Scan date | 2026-04-20 |
|
|
|
|
| Scan date | 2026-06-04 |
|
|
|
|
| Scan type | Full repository static analysis |
|
|
|
|
| Scan type | Full repository static analysis |
|
|
|
|
| Scope | `backend/`, `frontend/src/`, config files |
|
|
|
|
| Scope | `backend/server.js`, `backend/routes/`, `backend/middleware/`, `backend/helpers/`, `backend/scripts/`, `backend/migrations/`, `frontend/src/`, `.env.example` |
|
|
|
|
| Baseline | `docs/security-audit-2026-04-01.md` |
|
|
|
|
| Baseline | `docs/security-audit-2026-04-01.md` |
|
|
|
|
| Previous findings | 31 (6 Critical, 9 High, 10 Medium, 6 Low/Info) |
|
|
|
|
| Previous findings | 31 (6 Critical, 9 High, 10 Medium, 6 Low/Info) + 9 new (April 20) + 4 remediation plan items |
|
|
|
|
| Remediated | 20 fully fixed, 2 partially fixed |
|
|
|
|
| Fixed since last scan | 5 (N-2, N-6, M-6, RP-4, L-1) |
|
|
|
|
| Still open (from baseline) | 13 |
|
|
|
|
| Downgraded | 1 (M-10: Medium → Low, partially fixed) |
|
|
|
|
| New findings | 9 |
|
|
|
|
| Still open (from baseline) | 10 |
|
|
|
|
| Total open | 22 (1 High, 11 Medium, 10 Low) |
|
|
|
|
| Still open (from April 20) | 7 |
|
|
|
|
|
|
|
|
| New findings (June 4) | 2 |
|
|
|
|
|
|
|
|
| Total open | 18 (1 High, 8 Medium, 9 Low) |
|
|
|
|
|
|
|
|
| Regressions | 0 |
|
|
|
|
| Methodology | Static analysis — code review of all route handlers, middleware, helpers, and frontend components |
|
|
|
|
| Methodology | Static analysis — code review of all route handlers, middleware, helpers, and frontend components |
|
|
|
|
|