Security [MEDIUM]: Dispo workbook path unvalidated + image upload polyglot risk #54

New Issue

2026-04-16T22:05:12+02:00

Hartmut commented

2026-04-16 22:05:12 +02:00

Problem

(1) workbookPathSchema accepts any absolute path ending in .xlsx; consumer uses path.resolve() without base-directory check. Admin can trigger parsing of arbitrary .xlsx anywhere the process can read. ExcelJS has had zip-slip/XXE CVEs. (2) validateImageDataUrl checks only first 18 bytes — polyglot PNG/SVG files pass magic-byte check but contain HTML/SVG tail → stored XSS if ever served without strict MIME.

Evidence

packages/application/src/use-cases/dispo-import/read-workbook.ts:60-85 — path.resolve without allowlist
packages/api/src/lib/image-validation.ts — only 24-base64-char prefix check
packages/api/src/router/project-cover.ts:117 — Gemini-generated covers skip validateImageDataUrl entirely

Impact

(1) Admin (or compromised admin) can reach parser CVE on arbitrary filesystem. (2) Stored XSS via polyglot image.

Proposed Fix

(1) Require import path relative to configured IMPORT_DIR via path.relative(IMPORT_DIR, resolved).startsWith('..') check. Reject absolute paths from client. (2) Re-encode all uploaded images through sharp to canonical PNG/JPEG; never round-trip raw data URL. Apply validation to Gemini-generated covers too.

Acceptance Criteria

Dispo import: absolute path from client → 400
Image upload: re-encoded via sharp; tail HTML stripped
Test uploads polyglot → served safely

Parent Epic: #1
Source: Full-Codebase Security Audit 2026-04-16 (B-7, B-9)

## Problem (1) `workbookPathSchema` accepts any absolute path ending in `.xlsx`; consumer uses `path.resolve()` without base-directory check. Admin can trigger parsing of arbitrary `.xlsx` anywhere the process can read. ExcelJS has had zip-slip/XXE CVEs. (2) `validateImageDataUrl` checks only first 18 bytes — polyglot PNG/SVG files pass magic-byte check but contain HTML/SVG tail → stored XSS if ever served without strict MIME. ## Evidence - `packages/application/src/use-cases/dispo-import/read-workbook.ts:60-85 — path.resolve without allowlist` - `packages/api/src/lib/image-validation.ts — only 24-base64-char prefix check` - `packages/api/src/router/project-cover.ts:117 — Gemini-generated covers skip validateImageDataUrl entirely` ## Impact (1) Admin (or compromised admin) can reach parser CVE on arbitrary filesystem. (2) Stored XSS via polyglot image. ## Proposed Fix (1) Require import path relative to configured IMPORT_DIR via `path.relative(IMPORT_DIR, resolved).startsWith('..')` check. Reject absolute paths from client. (2) Re-encode all uploaded images through `sharp` to canonical PNG/JPEG; never round-trip raw data URL. Apply validation to Gemini-generated covers too. ## Acceptance Criteria - [ ] Dispo import: absolute path from client → 400 - [ ] Image upload: re-encoded via sharp; tail HTML stripped - [ ] Test uploads polyglot → served safely --- Parent Epic: #1 Source: Full-Codebase Security Audit 2026-04-16 (B-7, B-9)

Hartmut added the security label 2026-04-16 22:05:12 +02:00

Hartmut referenced this issue

2026-04-16 22:05:47 +02:00

CDP Compliance Epic — alle Controls #1

Hartmut referenced this issue from a commit

2026-04-17 15:26:31 +02:00

security: workbook path allowlist + stronger image polyglot validation (#54)

Hartmut commented

2026-04-17 15:27:03 +02:00

Resolved in commit c4b01c1bfc on branch security/audit-2026-04-17.

What changed

Dispo workbook path allowlist

New DISPO_IMPORT_DIR env var (defaults to ./imports) pins the root for workbook reads.
tRPC input schema (workbookPathSchema) rejects absolute paths and any segment equal to ...
read-workbook.ts re-validates containment at runtime using path.relative.
Closes the path-traversal class that would have let an admin (or compromised admin token) point ExcelJS at arbitrary files on disk.

Image upload / provider-image polyglot defence

validateImageDataUrl now checks the full 8-byte PNG magic, enforces PNG IEND and JPEG FFD9 trailers, scans the decoded buffer for markup polyglot markers (<script, <svg, <iframe, <object, <embed, <html, <!doctype, javascript:, onerror=, onload=), and explicitly rejects SVG.
Provider-generated covers (DALL-E, Gemini) run through the same validator before persistence.
Bounded decoder (16 MB max) for validation.

Tests

New packages/api/src/__tests__/image-validation.test.ts with 10 cases.
Application/API/web suites green (195 / 1932 / 1286).

Docs

docs/security-architecture.md §5 updated. .env.example documents DISPO_IMPORT_DIR.

Closing.

Resolved in commit c4b01c1bfc41605009a33910e458e03c51a33155 on branch `security/audit-2026-04-17`. ## What changed **Dispo workbook path allowlist** - New `DISPO_IMPORT_DIR` env var (defaults to `./imports`) pins the root for workbook reads. - tRPC input schema (`workbookPathSchema`) rejects absolute paths and any segment equal to `..`. - `read-workbook.ts` re-validates containment at runtime using `path.relative`. - Closes the path-traversal class that would have let an admin (or compromised admin token) point ExcelJS at arbitrary files on disk. **Image upload / provider-image polyglot defence** - `validateImageDataUrl` now checks the full 8-byte PNG magic, enforces PNG IEND and JPEG FFD9 trailers, scans the decoded buffer for markup polyglot markers (`<script`, `<svg`, `<iframe`, `<object`, `<embed`, `<html`, `<!doctype`, `javascript:`, `onerror=`, `onload=`), and explicitly rejects SVG. - Provider-generated covers (DALL-E, Gemini) run through the same validator before persistence. - Bounded decoder (16 MB max) for validation. ## Tests - New `packages/api/src/__tests__/image-validation.test.ts` with 10 cases. - Application/API/web suites green (195 / 1932 / 1286). ## Docs - `docs/security-architecture.md` §5 updated. `.env.example` documents `DISPO_IMPORT_DIR`. Closing.

Hartmut closed this issue

2026-04-17 15:27:07 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Hartmut/CapaKraken#54