diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 64a5233..051b4b8 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -21,3 +21,7 @@ **Vulnerability:** The plaintext password fallback logic incorrectly generated a recommended `scrypt` hash using the SHA256 hashed password rather than the raw `storedPassword`. This leads to generating an invalid `scrypt` hash that would fail authentication if the user updated their configuration with it. Furthermore, hashing variable-length secrets with SHA256 before `crypto.timingSafeEqual` prevents timing attacks, but using a non-keyed hash like SHA256 may leave it vulnerable. HMAC-SHA256 keyed with a secret provides better security. **Learning:** When generating a secure hash recommendation for users migrating from plaintext, ensure the raw input is used, not an intermediate hash intended for timing-safe comparison. **Prevention:** Generate the `scrypt` hash directly from the raw secret string (`storedPassword`). Always use HMAC keyed with a secret for hashing variable-length passwords prior to constant-time comparison. +## 2024-05-30 - Prevent Stored XSS via Content-Type Validation on Proxied Assets +**Vulnerability:** Asset proxy routes (`/api/image/*`, `/api/video/*`) directly served the upstream `Content-Type` header without validation. Since these API routes are explicitly excluded from the Next.js Content Security Policy (CSP) matcher, returning types like `image/svg+xml` or `text/html` would execute scripts directly in the user's browser, leading to Stored XSS. +**Learning:** Endpoints that serve user-controlled content and are not protected by global CSP headers require strict MIME-type validation. An attacker could upload an SVG containing malicious scripts and retrieve it via these proxy endpoints to bypass standard security controls. +**Prevention:** Always sanitize the `Content-Type` header on proxy endpoints. First convert it to lowercase, remap required generic types (like `application/octet-stream` to `image/jpeg`), and finally enforce a strict positive allowlist (`startsWith('image/')` and `!includes('svg')`) before falling back to `application/octet-stream`. diff --git a/app/api/image/[id]/route.ts b/app/api/image/[id]/route.ts index d2bdf09..8a8a4d0 100644 --- a/app/api/image/[id]/route.ts +++ b/app/api/image/[id]/route.ts @@ -81,10 +81,19 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ return NextResponse.json({ error: 'Asset not found' }, { status: 404 }); } + let contentType = result.contentType.toLowerCase(); + if (contentType.includes('application/octet-stream')) { + contentType = 'image/jpeg'; + } else if ( + !contentType.startsWith('image/') || + contentType.includes('svg') || + contentType.includes('xml') + ) { + contentType = 'application/octet-stream'; + } + const headers: Record = { - 'Content-Type': result.contentType.includes('application/octet-stream') - ? 'image/jpeg' - : result.contentType, + 'Content-Type': contentType, // Images are immutable once uploaded to Immich — cache aggressively 'Cache-Control': 'public, max-age=31536000, immutable', ETag: etag, diff --git a/app/api/video/[id]/route.ts b/app/api/video/[id]/route.ts index c258ba6..37118d0 100644 --- a/app/api/video/[id]/route.ts +++ b/app/api/video/[id]/route.ts @@ -44,8 +44,13 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{ return NextResponse.json({ error: 'Asset not found' }, { status: 404 }); } + let contentType = result.contentType.toLowerCase(); + if (!contentType.startsWith('video/') && !contentType.startsWith('audio/')) { + contentType = 'application/octet-stream'; + } + const headers: Record = { - 'Content-Type': result.contentType, + 'Content-Type': contentType, // Videos are immutable once uploaded — cache aggressively 'Cache-Control': 'public, max-age=31536000, immutable', 'Accept-Ranges': 'bytes', diff --git a/lib/__tests__/immich.test.ts b/lib/__tests__/immich.test.ts index 2f25fb9..97a0a56 100644 --- a/lib/__tests__/immich.test.ts +++ b/lib/__tests__/immich.test.ts @@ -14,6 +14,7 @@ vi.mock('../config', async () => { standaloneAlbums: ['album-1'], subpages: [], albumOverrides: { 'album-1': 'Override Name' }, + albumDescriptions: {}, cacheTtl: 0, }), };