From 483190e7c1017dee289616d3c539b761862d84ca Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 30 Mar 2026 23:42:40 +0000 Subject: [PATCH] fix: XSS in GitHubPanel markdown renderer and RouteCalculator profile bug Escape HTML entities before dangerouslySetInnerHTML in release notes renderer to prevent stored XSS via malicious GitHub release bodies. Fix RouteCalculator ignoring the profile parameter (was hardcoded to 'driving'). https://claude.ai/code/session_01SoQKcF5Rz9Y8Nzo4PzkxY8 --- AUDIT_FINDINGS.md | 13 +++++++++++++ client/src/components/Admin/GitHubPanel.tsx | 8 ++++++-- client/src/components/Map/RouteCalculator.ts | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/AUDIT_FINDINGS.md b/AUDIT_FINDINGS.md index 252ec48..65fff0f 100644 --- a/AUDIT_FINDINGS.md +++ b/AUDIT_FINDINGS.md @@ -249,6 +249,19 @@ |---|----------|------|---------|-------------|-----------------|--------| | AD-1 | **MEDIUM** | `server/src/routes/admin.ts` | 302-310 | Self-update endpoint runs `git pull` then `npm run build`. While admin-only and `npm install` uses `--ignore-scripts`, `npm run build` executes whatever is in the pulled package.json. A compromised upstream could execute arbitrary code. | Document as accepted risk for self-hosted self-update feature. Users should pin to specific versions. | DOCUMENTED | +### 1.9 Client-Side XSS + +| # | Severity | File | Line(s) | Description | Recommended Fix | Status | +|---|----------|------|---------|-------------|-----------------|--------| +| X-1 | **CRITICAL** | `client/src/components/Admin/GitHubPanel.tsx` | 66, 106 | `dangerouslySetInnerHTML` with `inlineFormat()` renders GitHub release notes as HTML without escaping. Malicious HTML in release notes could execute scripts. | Escape HTML entities before applying markdown-style formatting. Validate link URLs. | FIXED | +| X-2 | **LOW** | `client/src/components/Map/MapView.tsx` | divIcon | Uses `escAttr()` for HTML sanitization in divIcon strings. Properly mitigated. | OK | OK | + +### 1.10 Route Calculator Bug + +| # | Severity | File | Line(s) | Description | Recommended Fix | Status | +|---|----------|------|---------|-------------|-----------------|--------| +| RC-1 | **HIGH** | `client/src/components/Map/RouteCalculator.ts` | 16 | OSRM URL hardcodes `'driving'` profile, ignoring the `profile` parameter. Walking/cycling routes always return driving results. | Use the `profile` parameter in URL construction. | FIXED | + ### Additional Findings (from exhaustive scan) - **MEDIUM** — `server/src/index.ts:121-136`: Upload files (`/uploads/:type/:filename`) served without authentication. UUIDs are unguessable but this is security-through-obscurity. **REQUIRES MANUAL REVIEW** — adding auth would break shared trip image URLs. diff --git a/client/src/components/Admin/GitHubPanel.tsx b/client/src/components/Admin/GitHubPanel.tsx index 8db9d6f..2dc6dfa 100644 --- a/client/src/components/Admin/GitHubPanel.tsx +++ b/client/src/components/Admin/GitHubPanel.tsx @@ -72,11 +72,15 @@ export default function GitHubPanel() { } } + const escapeHtml = (str) => str.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"') const inlineFormat = (text) => { - return text + return escapeHtml(text) .replace(/\*\*(.+?)\*\*/g, '$1') .replace(/`(.+?)`/g, '$1') - .replace(/\[([^\]]+)\]\(([^)]+)\)/g, '$1') + .replace(/\[([^\]]+)\]\(([^)]+)\)/g, (_, label, url) => { + const safeUrl = url.startsWith('http://') || url.startsWith('https://') ? url : '#' + return `${label}` + }) } for (const line of lines) { diff --git a/client/src/components/Map/RouteCalculator.ts b/client/src/components/Map/RouteCalculator.ts index 6f79a97..8300aab 100644 --- a/client/src/components/Map/RouteCalculator.ts +++ b/client/src/components/Map/RouteCalculator.ts @@ -13,7 +13,7 @@ export async function calculateRoute( } const coords = waypoints.map((p) => `${p.lng},${p.lat}`).join(';') - const url = `${OSRM_BASE}/driving/${coords}?overview=full&geometries=geojson&steps=false` + const url = `${OSRM_BASE}/${profile}/${coords}?overview=full&geometries=geojson&steps=false` const response = await fetch(url, { signal }) if (!response.ok) {