From 1d785356621de342f7a3e48270e1711df3dd9a6c Mon Sep 17 00:00:00 2001 From: Till Heidrich Date: Tue, 1 Sep 2026 03:25:02 +0000 Subject: [PATCH] fix: findings from the code review of the mail and sharing work Two reviews, both dimensions. The serious ones, verified before fixing: Sessions never expired server-side and survived a password reset. `Max-Age` is a request to the browser, not a rule for the server, and `readSession` only checked the signature -- a leaked cookie stayed valid indefinitely, and the very reset meant to rescue a compromised account left the intruder logged in. A demoted admin also kept admin, because the role came from the cookie. Now the cookie carries a session epoch, `verifySession` checks the account against the database on every request and takes the role from there, and the epoch is bumped on password reset and on any role change. This was pre-existing, but the reset feature promised safety it did not deliver. "Allow download: off" did nothing for images with transparency -- exactly the cutouts and stickers this tool exists for. The view path skipped resizing when `has_alpha`, so the untouched original went out at full resolution, right-click saveable. Verified: now 1500x2000 instead of 2400x3200, and the download route returns 403. The share title went into outgoing mail HTML unescaped while the message was escaped -- a foreign link could be posted through the account's own SPF/DKIM-signed mailserver. Both escaped now, plus length limits. argon2 ran on every request for passphrase-protected links, including every image: 130 ms and 64 MiB each. A 40-image gallery would have walked into the memory limit, and a few dozen wrong guesses could have stalled the process. Checked once, then a short-lived signed cookie scoped to that one link. The passphrase moved from the query string to POST (it was landing in history and proxy logs) and wrong guesses are now rate limited. `days` accepted nonsense and quietly produced an unlimited link -- `Number('abc')` is NaN and `NaN > 0` is false. `[]` coerced to 0, same result. Now rejected outright, which matters when the pictures are of children. Rate limits keyed on `clientAddress`, which is the *first* X-Forwarded-For value -- set by the caller. Counting from the right instead, with the number of trusted proxies configurable (TRUSTED_PROXIES, default 1), plus per-username and per-recipient counters, a real limit on outgoing mail, and a limiter that expires entries individually instead of clearing everyone's counter. Also: expired links could still be mailed; missing PUBLIC_BASE_URL sent dead links while reporting success; the reset endpoint's response time revealed whether an account existed (send is no longer awaited); invalid UUIDs produced 500s; the email uniqueness race returned an HTML error page; share_views grew without bound and is now one row per visitor per hour, hashed per link, user agent reduced to browser and system, deleted after 90 days; double provenance marking appended a second XMP packet with the older model instead of replacing. 21 new tests (97 total) for shares, mail and provenance -- all three were completely untested, and that is where both reviews found the most. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BNhLunz586g6DeYMa7Qsd1 --- migrations/015_session_epoch.sql | 37 +++++++ src/components/KontoApp.tsx | 5 +- src/components/ShareDialog.tsx | 2 +- src/components/SharesApp.tsx | 31 ++++-- src/lib/auth.ts | 62 ++++++++++-- src/lib/limit.ts | 72 ++++++++++++++ src/lib/mail.ts | 20 +++- src/lib/maintenance.ts | 19 ++++ src/lib/provenance.ts | Bin 6374 -> 8406 bytes src/lib/shares.ts | 161 ++++++++++++++++++++++++++----- src/middleware.ts | 7 +- src/pages/api/admin/users.ts | 7 +- src/pages/api/auth/login.ts | 21 +++- src/pages/api/auth/reset.ts | 37 +++---- src/pages/api/me.ts | 10 +- src/pages/api/s/[slug]/file.ts | 39 ++++++-- src/pages/api/shares/index.ts | 37 ++++--- src/pages/api/shares/send.ts | 27 +++++- src/pages/s/[slug].astro | 60 ++++++++++-- tests/mail.test.ts | 66 +++++++++++++ tests/provenance.test.ts | 38 ++++++++ tests/shares.test.ts | 102 ++++++++++++++++++++ 22 files changed, 752 insertions(+), 108 deletions(-) create mode 100644 migrations/015_session_epoch.sql create mode 100644 src/lib/limit.ts create mode 100644 tests/mail.test.ts create mode 100644 tests/shares.test.ts diff --git a/migrations/015_session_epoch.sql b/migrations/015_session_epoch.sql new file mode 100644 index 0000000..8ac634e --- /dev/null +++ b/migrations/015_session_epoch.sql @@ -0,0 +1,37 @@ +-- Sitzungen serverseitig entwertbar machen --------------------------------- +-- Bis hierher war das Sitzungs-Cookie nur HMAC-signiert: Wer es einmal hatte, +-- kam damit unbegrenzt hinein — auch nachdem das Passwort zurückgesetzt oder +-- die Rolle entzogen wurde. Genau der Vorgang, der ein übernommenes Konto +-- retten soll, half also nicht. +-- +-- `session_epoch` wird bei jedem Passwortwechsel und jeder Rollenänderung +-- hochgezählt. Das Cookie trägt den Stand mit; stimmt er nicht mehr, gilt es +-- nicht mehr. Ein Zähler statt eines Zeitstempels, weil er keine Uhrzeit- +-- Ungenauigkeiten kennt. +ALTER TABLE users ADD COLUMN IF NOT EXISTS session_epoch integer NOT NULL DEFAULT 0; + +-- Für die Namensvergabe: `renames.ts` fragt bei jedem fertigen Bild, ob ein +-- Name schon vergeben ist — ohne Index war das ein voller Tabellendurchlauf. +CREATE INDEX IF NOT EXISTS items_filename_idx ON items(filename); + +-- Aufräumen: Beide Tabellen wuchsen unbegrenzt. +CREATE INDEX IF NOT EXISTS share_views_at_idx ON share_views(at); +CREATE INDEX IF NOT EXISTS password_resets_expires_idx ON password_resets(expires_at); + +-- Zähler für verschiedene Besucher je Teilen-Link. Vorher wurde er bei jedem +-- Aufruf der Verwaltungsseite über `count(DISTINCT ip_hash)` aus share_views +-- gerechnet — das wurde mit wachsender Tabelle zäh und machte die Seite +-- angreifbar, weil jeder mit dem Link beliebig viele Zeilen erzeugen konnte. +ALTER TABLE shares ADD COLUMN IF NOT EXISTS besucher integer NOT NULL DEFAULT 0; + +-- Nur eine Zeile je Link, Besucher und Stunde: Ohne diese Grenze konnte ein +-- Empfänger (oder ein -Tag auf irgendeiner Seite) die Tabelle unbegrenzt +-- füllen und damit die Platte volllaufen lassen. +-- +-- Der Stundenwert wird beim Schreiben mitgegeben statt im Index gerechnet: +-- `date_trunc` auf einem timestamptz hängt an der Zeitzonen-Einstellung und ist +-- damit nicht unveränderlich genug für einen Index. +ALTER TABLE share_views ADD COLUMN IF NOT EXISTS stunde text; +CREATE UNIQUE INDEX IF NOT EXISTS share_views_einmal_je_stunde_idx + ON share_views (share_id, kind, ip_hash, stunde) + WHERE ip_hash IS NOT NULL AND stunde IS NOT NULL; diff --git a/src/components/KontoApp.tsx b/src/components/KontoApp.tsx index ccb09f6..5df300c 100644 --- a/src/components/KontoApp.tsx +++ b/src/components/KontoApp.tsx @@ -24,6 +24,9 @@ export default function KontoApp() { const sagen = (t: string, ok: boolean) => { setMsg({ t, ok }); setTimeout(() => setMsg(null), 5000); }; const patch = async (body: unknown, erfolg: string) => { + // Den neuen Wert sofort übernehmen, sonst springt der Haken beim Rerender + // auf den alten Stand zurück und man klickt aus Versehen ein zweites Mal. + setMe((m) => (m ? { ...m, ...(body as object) } as Me : m)); setLaeuft(true); const r = await fetch('/api/me', { method: 'PATCH', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), @@ -89,7 +92,7 @@ export default function KontoApp() {
Benachrichtigungen
@@ -158,8 +167,10 @@ export default function SharesApp() { ))} )} -

Es wird festgehalten, wann und womit geöffnet wurde — - nicht wer. Die IP-Adresse liegt nur gehasht vor und lässt sich nicht zurückrechnen.

+

Festgehalten wird, wann und mit welcher Art Gerät geöffnet + wurde — nicht wer. Von der IP-Adresse wird nur ein verschlüsselter Fingerabdruck + gespeichert, der außerdem an diesen einen Link gebunden ist; über mehrere Links hinweg + lässt sich damit niemand verfolgen. Nach 90 Tagen wird alles gelöscht.

)} diff --git a/src/lib/auth.ts b/src/lib/auth.ts index c3841d2..5a329e1 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,8 +1,10 @@ import argon2 from 'argon2'; import { createHmac, timingSafeEqual } from 'node:crypto'; -import { one } from './db'; +import { one, query } from './db'; export interface SessionUser { uid: string; role: 'user' | 'admin'; name: string; } +/** Was tatsächlich im Cookie steht — inklusive der Angaben, die es entwertbar machen. */ +interface SessionPayload extends SessionUser { iat: number; ep: number } const COOKIE = 'kb_session'; const MAX_AGE = 60 * 60 * 24 * 30; // 30 Tage @@ -20,8 +22,8 @@ function sign(data: string): string { return createHmac('sha256', secret()).update(data).digest('base64url'); } -export function makeSessionCookie(u: SessionUser): string { - const payload = Buffer.from(JSON.stringify({ ...u, iat: Math.floor(Date.now() / 1000) })) +export function makeSessionCookie(u: SessionUser, epoch = 0): string { + const payload = Buffer.from(JSON.stringify({ ...u, iat: Math.floor(Date.now() / 1000), ep: epoch })) .toString('base64url'); const value = `${payload}.${sign(payload)}`; return `${COOKIE}=${value}; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=${MAX_AGE}`; @@ -30,7 +32,18 @@ export function makeSessionCookie(u: SessionUser): string { export const clearSessionCookie = () => `${COOKIE}=; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=0`; -export function readSession(cookieHeader?: string | null): SessionUser | null { +/** + * Cookie prüfen — Signatur **und** Alter. + * + * Die Alterprüfung ist nicht Kosmetik: `Max-Age` ist eine Bitte an den Browser, + * keine Regel für den Server. Ohne sie gilt ein einmal abgeflossenes Cookie + * unbegrenzt, auch Jahre später und auch per curl. + * + * Ob das Cookie **inhaltlich** noch gilt (Passwort gewechselt, Rolle entzogen, + * Konto gelöscht), lässt sich hier nicht entscheiden — dafür braucht es die + * Datenbank. Das erledigt `verifySession` in der Middleware. + */ +export function readSession(cookieHeader?: string | null): (SessionUser & { ep: number }) | null { if (!cookieHeader) return null; const raw = cookieHeader.split(/;\s*/).find((c) => c.startsWith(`${COOKIE}=`))?.slice(COOKIE.length + 1); if (!raw) return null; @@ -40,19 +53,48 @@ export function readSession(cookieHeader?: string | null): SessionUser | null { const a = Buffer.from(sig), b = Buffer.from(expected); if (a.length !== b.length || !timingSafeEqual(a, b)) return null; try { - const obj = JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')); - return { uid: obj.uid, role: obj.role, name: obj.name }; + const obj = JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')) as SessionPayload; + if (!obj.uid || typeof obj.iat !== 'number') return null; + if (Math.floor(Date.now() / 1000) - obj.iat > MAX_AGE) return null; // serverseitig abgelaufen + return { uid: obj.uid, role: obj.role, name: obj.name, ep: Number(obj.ep) || 0 }; } catch { return null; } } -export async function login(username: string, password: string): Promise { - const u = await one<{ id: string; password_hash: string; role: 'user' | 'admin'; display_name: string }>( - 'SELECT id, password_hash, role, display_name FROM users WHERE username=$1', [username]); +/** + * Gegenprobe in der Datenbank: Gibt es das Konto noch, gilt der Sitzungsstand + * noch, und welche Rolle hat es **jetzt**? + * + * Die Rolle kommt bewusst aus der Datenbank und nicht aus dem Cookie — sonst + * behielte ein herabgestufter Admin seine Rechte bis zum Ablauf des Cookies. + * Ein Punkt-Lookup über den Primärschlüssel, den `tokenUser` ohnehin macht. + */ +export async function verifySession(s: (SessionUser & { ep: number }) | null): Promise { + if (!s?.uid) return null; + const u = await one<{ role: 'user' | 'admin'; display_name: string | null; username: string; session_epoch: number }>( + 'SELECT role, display_name, username, session_epoch FROM users WHERE id = $1', [s.uid]); + if (!u) return null; // Konto gelöscht + if ((u.session_epoch ?? 0) !== s.ep) return null; // Passwort gewechselt oder Rolle geändert + return { uid: s.uid, role: u.role, name: u.display_name || u.username }; +} + +/** Alle Sitzungen eines Kontos entwerten — nach Passwortwechsel oder Rollenänderung. */ +export async function bumpSessionEpoch(userId: string): Promise { + await query('UPDATE users SET session_epoch = session_epoch + 1 WHERE id = $1', [userId]); +} + +export async function login( + username: string, password: string, +): Promise<{ user: SessionUser; epoch: number } | null> { + const u = await one<{ id: string; password_hash: string; role: 'user' | 'admin'; display_name: string; username: string; session_epoch: number }>( + 'SELECT id, password_hash, role, display_name, username, session_epoch FROM users WHERE username=$1', [username]); if (!u) return null; if (!(await verifyPassword(u.password_hash, password))) return null; - return { uid: u.id, role: u.role, name: u.display_name || username }; + return { + user: { uid: u.id, role: u.role, name: u.display_name || u.username }, + epoch: u.session_epoch ?? 0, + }; } // --- einfacher In-Memory-Ratelimiter: max 5 Fehlversuche / 15 Min pro IP --- diff --git a/src/lib/limit.ts b/src/lib/limit.ts new file mode 100644 index 0000000..a8a1246 --- /dev/null +++ b/src/lib/limit.ts @@ -0,0 +1,72 @@ +// Ratenbegrenzung und die Frage, wem man die Absender-Adresse glaubt. +// +// Bewusst ohne Abhängigkeit und im Speicher: Klarbild läuft als **ein** Node- +// Prozess. Käme je ein zweiter dazu, gehört das in die Datenbank oder nach +// Redis — dann ist diese Datei die eine Stelle, die man austauscht. + +/** + * Die tatsächliche Adresse des Aufrufers. + * + * Wichtig und leicht zu übersehen: Astros `clientAddress` nimmt den **ersten** + * Wert aus `X-Forwarded-For`. Diesen Wert schickt der Angreifer selbst — ein + * Reverse Proxy hängt die echte Adresse hinten an. Jede Begrenzung, die auf + * `clientAddress` baut, lässt sich damit umgehen, indem man den Header pro + * Anfrage variiert. + * + * Deshalb wird von rechts gezählt: Bei `TRUSTED_PROXIES=1` (Standard, ein + * vorgelagerter Proxy wie Traefik) ist der letzte Eintrag der, den der Proxy + * selbst gesetzt hat — den kann niemand von außen fälschen. Ohne Proxy + * (`TRUSTED_PROXIES=0`) wird der Header gar nicht beachtet. + */ +export function echteIp(request: Request, fallback?: string | null): string { + const vertraut = Number(process.env.TRUSTED_PROXIES ?? '1'); + if (!Number.isFinite(vertraut) || vertraut <= 0) return fallback || 'unbekannt'; + const kette = (request.headers.get('x-forwarded-for') || '') + .split(',').map((s) => s.trim()).filter(Boolean); + if (!kette.length) return fallback || 'unbekannt'; + // Von rechts so viele Sprünge zurück, wie es vertrauenswürdige Proxys gibt. + const i = Math.max(0, kette.length - vertraut); + return kette[i] || fallback || 'unbekannt'; +} + +interface Eintrag { n: number; bis: number } +const eimer = new Map(); +let letzteReinigung = 0; + +/** Abgelaufene Einträge entfernen — gezielt, nicht alles auf einmal. */ +function aufraeumen(jetzt: number): void { + if (jetzt - letzteReinigung < 60_000) return; + letzteReinigung = jetzt; + for (const [k, e] of eimer) if (e.bis < jetzt) eimer.delete(k); + // Notbremse, falls sich trotzdem zu viel ansammelt: die ältesten zuerst. + if (eimer.size > 50_000) { + const nach = [...eimer.entries()].sort((a, b) => a[1].bis - b[1].bis); + for (const [k] of nach.slice(0, eimer.size - 25_000)) eimer.delete(k); + } +} + +/** + * Einen Versuch zählen. Liefert `true`, wenn die Grenze überschritten ist. + * + * Anders als vorher wird hier **nicht** die ganze Tabelle geleert, wenn sie + * groß wird — damit hätte ein Angreifer die Zähler aller anderen mit + * zurückgesetzt. + */ +export function zuOft(schluessel: string, max: number, fensterMs: number): boolean { + const jetzt = Date.now(); + aufraeumen(jetzt); + const e = eimer.get(schluessel); + if (!e || e.bis < jetzt) { eimer.set(schluessel, { n: 1, bis: jetzt + fensterMs }); return false; } + e.n++; + return e.n > max; +} + +/** Zähler zurücksetzen — nach einem erfolgreichen Vorgang. */ +export const zuruecksetzen = (schluessel: string): void => { eimer.delete(schluessel); }; + +/** Wie viele Versuche im laufenden Fenster noch frei sind. */ +export function restVersuche(schluessel: string, max: number): number { + const e = eimer.get(schluessel); + if (!e || e.bis < Date.now()) return max; + return Math.max(0, max - e.n); +} diff --git a/src/lib/mail.ts b/src/lib/mail.ts index 1462480..4d8b213 100644 --- a/src/lib/mail.ts +++ b/src/lib/mail.ts @@ -43,7 +43,7 @@ export interface MailInput { html?: string; } -const esc = (s: string) => +export const esc = (s: string) => s.replace(/&/g, '&').replace(//g, '>').replace(/"/g, '"'); /** @@ -126,6 +126,18 @@ export async function testMail(): Promise<{ ok: boolean; message: string }> { } } -/** Grobe Plausibilitätsprüfung einer Adresse. */ -export const istMail = (v: string): boolean => - /^[^\s@]+@[^\s@.]+(\.[^\s@.]+)+$/.test(String(v || '').trim()) && String(v).length <= 254; +/** + * Grobe Plausibilitätsprüfung einer Adresse. + * + * Steuerzeichen werden am **rohen** Wert geprüft, nicht am getrimmten: In + * JavaScript passt `$` ohne `m`-Flag auch vor einem abschließenden + * Zeilenumbruch, und `trim()` würde ihn ohnehin wegnehmen. Beides zusammen + * hätte `"a@b.de\n"` durchgelassen — und genau damit fängt jede + * Kopfzeilen-Einschleusung an. + */ +export const istMail = (v: string): boolean => { + const roh = String(v ?? ''); + if (/[\r\n\t\0]/.test(roh)) return false; + const s = roh.trim(); + return !!s && s.length <= 254 && /^[^\s@]+@[^\s@.]+(\.[^\s@.]+)+$/.test(s); +}; diff --git a/src/lib/maintenance.ts b/src/lib/maintenance.ts index 751f078..1c5ba20 100644 --- a/src/lib/maintenance.ts +++ b/src/lib/maintenance.ts @@ -110,6 +110,23 @@ export async function purgePrivate(olderThanMin = 60): Promise<{ deleted: number return { deleted: rows.length }; } +/** + * Aufzeichnungen und abgelaufene Rücksetz-Links wegräumen. + * + * Beide Tabellen wuchsen vorher unbegrenzt. Bei `share_views` ist das nicht nur + * eine Platzfrage: Es sind Aufzeichnungen darüber, wann jemand ein Bild + * angesehen hat. Sie länger aufzuheben, als man sie braucht, wäre schlicht + * unnötig — 90 Tage reichen für „wurde das eigentlich angeschaut?". + */ +export async function purgeTracking(tage = 90): Promise<{ views: number; resets: number }> { + const v = await query<{ id: string }>( + `DELETE FROM share_views WHERE at < now() - ($1 || ' days')::interval RETURNING id`, [String(tage)]); + const r = await query<{ token_hash: string }>( + `DELETE FROM password_resets + WHERE expires_at < now() - interval '7 days' RETURNING token_hash`); + return { views: v.length, resets: r.length }; +} + let timer: NodeJS.Timeout | null = null; /** Wartungs-Lauf: Retention + private Sessions vergessen. */ export function startMaintenance(): void { @@ -123,6 +140,8 @@ export function startMaintenance(): void { } const p = await purgePrivate(60); if (p.deleted) console.log(`[maintenance] Privat: ${p.deleted} Ergebnisse vergessen.`); + const t = await purgeTracking(90); + if (t.views || t.resets) console.log(`[maintenance] Aufgeräumt: ${t.views} Zugriffe, ${t.resets} alte Rücksetz-Links.`); } catch (e) { console.error('[maintenance]', e); } }; timer = setInterval(tick, 30 * 60 * 1000); // alle 30 min (auch für private Purge) diff --git a/src/lib/provenance.ts b/src/lib/provenance.ts index 77f13be59a6da722c62f757caa17a6df3e0661da..96de339029127380c8d68c27188b6928a30eb79a 100644 GIT binary patch delta 1887 zcmbtUPj4GV6c>urL*PWf76<4ez==eKVv51QM-J5QqJVD|hM~(O1~8dj3l|~P&|lGUOiC(2#p+d^m*7O_;YmDc`+&yG@`} zf)_PEm96%ZWmuH?cAt%q!k}5mlY(mh_+UGDd@8}2PHgzP-#ggu_WMoC&R�xWT&( zdum8+R`CHB>)GF>dU~;4o6Xh@=*m<>4haKx_E|ScIahy=iPQEbl zsTrzzZ3~($_@V>4IYjPB-=gF6+yLa>(kgY*ergM3?stb=-f!*oboboEqEeoZ^GaLB zClmzts+Ew*fHy03LtqOY6dyaU_3D_Zc|mCl=-iwwfKCZ{N)=z;AnFjIQBjn$4H8N5 z6OtiAc~-RPmAN9&$!e+pNpNRDP+RlRGBZ&*J$lq0q;X_3H+Imb`Kc7gYHpwAr&?fA zvBxU$^#Cr@AX#ZT4~3uts*U0ANG#-D;BIMyj&v@S@Xm4d*&1abh)0j%e8}x zw;PPs?ZIei64|>OyV?H6XCLQ|M~t32x>V*;xU3+Py?Vdgq(_B0;0LrniX;6R=vQSg z2wnZ^^10nB1C;%~@kzQIsvB{)$m&~Zzg5^bsA6+|{Of6_Z&6;{n2XL0P~lJrfc`*ujPmD{0ouKQLF#} delta 73 zcmccS_{?xa2J_@v79B>0&Ffhlg(o{mDo$Q5r8c=sQh+z9G)+gLBDX-(noB`p@_QMr Z$+sj0nSr8{KS~ONX)Y 0` ist `false` — mit der naiven + * Schreibweise landete jede unsinnige Eingabe im Zweig „unbegrenzt". Genau die + * Voreinstellung, die diese Datei oben als falsch bezeichnet. Deshalb: alles, + * was keine sinnvolle Zahl ist, wird abgelehnt statt stillschweigend gedeutet. + */ +export function tageLesen(v: unknown, standard: number): number { + if (v === undefined || v === null || v === '') return standard; + // Nur Zahl oder Zeichenkette annehmen: `Number([])` ist 0 und `Number([7])` + // ist 7 — ein versehentlich mitgeschicktes Array würde sonst stillschweigend + // zu „unbegrenzt" oder zu einer erfundenen Frist. + if (typeof v !== 'number' && typeof v !== 'string') + throw new Error('Gültigkeit muss eine Zahl von Tagen sein.'); + const n = Number(v); + if (!Number.isFinite(n)) throw new Error('Gültigkeit muss eine Zahl von Tagen sein.'); + const t = Math.trunc(n); + if (t < 0 || t > MAX_TAGE) throw new Error(`Gültigkeit muss zwischen 0 und ${MAX_TAGE} Tagen liegen.`); + return t; +} + +const kurz = (v: unknown, max: number): string | null => { + const t = String(v ?? '').trim(); + return t ? t.slice(0, max) : null; +}; /** * Link anlegen. Ohne ausdrückliche Angabe gilt die Voreinstellung aus den @@ -60,7 +90,7 @@ export async function createShare(input: ShareErstellen): Promise { if (!ids.length) throw new Error('Keine Bilder ausgewählt.'); const s = await one<{ share_default_days: number }>('SELECT share_default_days FROM settings WHERE id=1'); - const tage = input.days === undefined || input.days === null ? (s?.share_default_days ?? 30) : Number(input.days); + const tage = tageLesen(input.days, s?.share_default_days ?? 30); const expires = tage > 0 ? new Date(Date.now() + tage * 86400_000) : null; const kind: ShareKind = input.kind === 'collection' || ids.length > 1 ? 'collection' : 'item'; const pw = input.password?.trim() ? await argon2.hash(input.password.trim()) : null; @@ -68,12 +98,22 @@ export async function createShare(input: ShareErstellen): Promise { const share = await one( `INSERT INTO shares (slug, kind, title, note, created_by, expires_at, allow_download, password_hash) VALUES ($1,$2,$3,$4,$5,$6,$7,$8) RETURNING *`, - [neuerSlug(), kind, input.title?.trim() || null, input.note?.trim() || null, + [neuerSlug(), kind, kurz(input.title, MAX_TITEL), kurz(input.note, MAX_NOTIZ), input.userId, expires, input.allowDownload !== false, pw]); - for (const [i, id] of ids.entries()) { - await query('INSERT INTO share_items (share_id, item_id, position) VALUES ($1,$2,$3) ON CONFLICT DO NOTHING', - [share!.id, id, i]); + // In **einem** Schritt einfügen: Vorher waren es bis zu 500 einzelne Abfragen, + // und wenn eine davon scheiterte (etwa weil ein Bild zwischenzeitlich gelöscht + // wurde), blieb ein halb gefüllter Link stehen, während die Oberfläche einen + // Fehler meldete. + try { + await query( + `INSERT INTO share_items (share_id, item_id, position) + SELECT $1, x.id, x.pos FROM unnest($2::uuid[]) WITH ORDINALITY AS x(id, pos) + ON CONFLICT DO NOTHING`, [share!.id, ids]); + } catch (e) { + // Kein halber Link: Was nicht vollständig entstehen konnte, wird entfernt. + await query('DELETE FROM shares WHERE id = $1', [share!.id]).catch(() => {}); + throw e; } return share!; } @@ -103,20 +143,18 @@ export async function resolveShare(slug: string, passwort?: string | null): Prom } export interface ShareBild { - id: string; filename: string | null; output_px: string | null; - mode: string | null; created_at: string; + id: string; filename: string | null; output_px: string | null; created_at: string; } /** Die Bilder hinter einem Link, in der gespeicherten Reihenfolge. */ export async function shareItems(shareId: string): Promise { return query( - // `mode` hängt am Auftrag, nicht am Bild. Und es muss maskiert werden: - // Postgres liest `i.mode` sonst als Funktionsaufruf `mode(i)` — das ist die - // Aggregatfunktion, daher die Meldung „WITHIN GROUP is required". - `SELECT i.id, i.filename, i.output_px, j."mode" AS mode, i.created_at + // Kein Join auf `jobs`: `items.job_id` ist laut Schema nullable, ein INNER + // JOIN könnte also Zeilen verschlucken — und gebraucht wurde davon ohnehin + // nichts. + `SELECT i.id, i.filename, i.output_px, i.created_at FROM share_items si JOIN items i ON i.id = si.item_id - JOIN jobs j ON j.id = i.job_id WHERE si.share_id = $1 AND i.status = 'done' AND i.result_path IS NOT NULL ORDER BY si.position, i.created_at`, [shareId]); } @@ -137,21 +175,53 @@ export async function trackShare( shareId: string, kind: 'view' | 'download', ip?: string | null, ua?: string | null, ): Promise { const salz = process.env.SESSION_SECRET || 'klarbild'; - const ipHash = ip ? createHash('sha256').update(salz + '|' + ip).digest('base64url').slice(0, 22) : null; + // Der Hash bindet die Kennung des Links mit ein: Damit lässt sich derselbe + // Besucher **innerhalb** eines Links wiedererkennen, aber nicht über mehrere + // Links hinweg verfolgen. (Ehrlich gesagt: Das ist Pseudonymisierung, keine + // Anonymisierung — wer den Server-Schlüssel hat, kann den IPv4-Raum + // durchrechnen. Deshalb wird auch nur 90 Tage aufgehoben.) + const ipHash = ip + ? createHash('sha256').update(`${salz}|${shareId}|${ip}`).digest('base64url').slice(0, 22) + : null; + const stunde = new Date().toISOString().slice(0, 13); // „2026-09-01T14" try { - await query('INSERT INTO share_views (share_id, kind, ip_hash, user_agent) VALUES ($1,$2,$3,$4)', - [shareId, kind, ipHash, (ua || '').slice(0, 200) || null]); + // Höchstens eine Zeile je Besucher und Stunde — sonst füllt ein einzelner + // Empfänger (oder ein -Tag auf einer fremden Seite) die Tabelle. + const neu = await query<{ id: string }>( + `INSERT INTO share_views (share_id, kind, ip_hash, user_agent, stunde) + VALUES ($1,$2,$3,$4,$5) ON CONFLICT DO NOTHING RETURNING id`, + [shareId, kind, ipHash, geraet(ua), stunde]); + + // Der Zähler zählt jeden Aufruf, die Besucherzahl nur neue Kennungen. await query( `UPDATE shares SET last_seen_at = now(), - views = views + CASE WHEN $2='view' THEN 1 ELSE 0 END, - downloads = downloads + CASE WHEN $2='download' THEN 1 ELSE 0 END - WHERE id = $1`, [shareId, kind]); + views = views + CASE WHEN $2='view' THEN 1 ELSE 0 END, + downloads = downloads + CASE WHEN $2='download' THEN 1 ELSE 0 END, + besucher = besucher + CASE WHEN $3 AND $2='view' THEN 1 ELSE 0 END + WHERE id = $1`, [shareId, kind, neu.length > 0]); } catch (e: any) { // Zählen ist Beiwerk — es darf die Auslieferung nie verhindern. console.error('[share] Zugriff nicht gezählt:', e?.message || e); } } +/** + * Aus dem User-Agent nur Browser und System behalten. + * Die vollständige Zeichenkette ist ein brauchbarer Fingerabdruck; für „womit + * wurde geöffnet" reicht „Safari · iPhone" völlig. + */ +export function geraet(ua?: string | null): string | null { + const s = String(ua || ''); + if (!s) return null; + const browser = /Edg\//.test(s) ? 'Edge' : /OPR\//.test(s) ? 'Opera' + : /Chrome\//.test(s) ? 'Chrome' : /Firefox\//.test(s) ? 'Firefox' + : /Safari\//.test(s) ? 'Safari' : 'Browser'; + const system = /iPhone/.test(s) ? 'iPhone' : /iPad/.test(s) ? 'iPad' + : /Android/.test(s) ? 'Android' : /Macintosh|Mac OS/.test(s) ? 'Mac' + : /Windows/.test(s) ? 'Windows' : /Linux/.test(s) ? 'Linux' : ''; + return system ? `${browser} · ${system}` : browser; +} + export interface ShareStat extends Share { anzahl: number; besucher: number; } @@ -159,9 +229,14 @@ export interface ShareStat extends Share { /** Übersicht für den Eigentümer (Admins sehen alle). */ export async function listShares(userId: string | null, istAdmin: boolean): Promise { return query( + // `anzahl` zählt dieselben Bedingungen wie `shareItems` — sonst nennt die + // Verwaltung eine andere Zahl als die Seite, die der Empfänger sieht. + // `besucher` kommt aus der gepflegten Spalte statt aus einer Aggregation + // über share_views: Bei vielen Zugriffen wurde diese Seite sonst zäh. `SELECT s.*, - (SELECT count(*)::int FROM share_items si WHERE si.share_id = s.id) AS anzahl, - (SELECT count(DISTINCT ip_hash)::int FROM share_views v WHERE v.share_id = s.id) AS besucher + (SELECT count(*)::int FROM share_items si JOIN items i ON i.id = si.item_id + WHERE si.share_id = s.id AND i.status='done' AND i.result_path IS NOT NULL) AS anzahl, + s.besucher FROM shares s WHERE $2::boolean OR s.created_by = $1 ORDER BY s.created_at DESC LIMIT 200`, [userId, istAdmin]); @@ -200,11 +275,11 @@ export async function updateShare( const sets: string[] = []; const args: any[] = [id]; const set = (col: string, val: any) => { args.push(val); sets.push(`${col}=$${args.length}`); }; - if (a.title !== undefined) set('title', a.title?.trim() || null); - if (a.note !== undefined) set('note', a.note?.trim() || null); + if (a.title !== undefined) set('title', kurz(a.title, MAX_TITEL)); + if (a.note !== undefined) set('note', kurz(a.note, MAX_NOTIZ)); if (a.allowDownload !== undefined) set('allow_download', !!a.allowDownload); if (a.days !== undefined) { - const t = Number(a.days) || 0; + const t = tageLesen(a.days, 0); set('expires_at', t > 0 ? new Date(Date.now() + t * 86400_000) : null); } if (a.password !== undefined) { @@ -221,9 +296,45 @@ export async function deleteShare(id: string, userId: string | null, istAdmin: b return r.length > 0; } -/** Zeitkonstanter Vergleich — für Stellen, an denen ein Zeitunterschied verriete, wie weit man richtig lag. */ +/** Zeitkonstanter Vergleich — verrät nicht über die Dauer, wie weit man richtig lag. */ export function gleich(a: string, b: string): boolean { const x = Buffer.from(String(a)); const y = Buffer.from(String(b)); if (x.length !== y.length) return false; return timingSafeEqual(x, y); } + +/* ---------------- Nachweis für ein geprüftes Zugangswort ---------------- + * `argon2.verify` kostet rund 130 ms und 64 MiB — und lief vorher bei **jeder** + * Anfrage, also auch für jedes einzelne Bild einer Galerie. Eine Seite mit 40 + * Bildern hätte den Prozess in die Speichergrenze getrieben, und ein paar + * Dutzend Anfragen mit falschem Wort hätten die ganze Anwendung lahmgelegt. + * + * Deshalb: einmal prüfen, danach einen kurzlebigen, signierten Nachweis + * mitführen, der **nur für diesen einen Link** gilt. */ + +const NACHWEIS_DAUER = 6 * 60 * 60; // 6 Stunden + +export const nachweisName = (slug: string) => `kb_s_${slug}`; + +export function nachweisBauen(shareId: string): string { + const bis = Math.floor(Date.now() / 1000) + NACHWEIS_DAUER; + const sig = createHmac('sha256', process.env.SESSION_SECRET || 'klarbild') + .update(`${shareId}.${bis}`).digest('base64url'); + return `${bis}.${sig}`; +} + +export function nachweisGilt(shareId: string, wert?: string | null): boolean { + if (!wert) return false; + const [bisRoh, sig] = String(wert).split('.'); + const bis = Number(bisRoh); + if (!Number.isFinite(bis) || bis < Math.floor(Date.now() / 1000)) return false; + const soll = createHmac('sha256', process.env.SESSION_SECRET || 'klarbild') + .update(`${shareId}.${bis}`).digest('base64url'); + return !!sig && gleich(sig, soll); +} + +/** Nachweis aus dem Cookie-Kopf ziehen. */ +export function nachweisAusCookie(cookieHeader: string | null | undefined, slug: string): string | null { + const name = nachweisName(slug); + return cookieHeader?.split(/;\s*/).find((c) => c.startsWith(`${name}=`))?.slice(name.length + 1) || null; +} diff --git a/src/middleware.ts b/src/middleware.ts index f2ceaf9..fbe84c4 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -1,6 +1,6 @@ import { defineMiddleware } from 'astro:middleware'; import { ensureInit } from './lib/init'; -import { readSession } from './lib/auth'; +import { readSession, verifySession } from './lib/auth'; import { one } from './lib/db'; const PUBLIC_PATHS = [ @@ -27,7 +27,10 @@ export const onRequest = defineMiddleware(async (ctx, next) => { // Health/Webhook dürfen laufen, auch wenn Init noch hakt — sonst blockiert nichts. try { await ensureInit(); } catch (e) { if (ctx.url.pathname !== '/api/health') throw e; } - let user = readSession(ctx.request.headers.get('cookie')); + // Signatur und Alter prüft `readSession`; ob das Konto noch existiert, die + // Rolle noch stimmt und der Sitzungsstand noch gilt, entscheidet erst die + // Datenbank — sonst überlebte ein Cookie jeden Passwortwechsel. + let user = await verifySession(readSession(ctx.request.headers.get('cookie'))); // Programmatischer Zugriff per API-Token (nur für /api/*, außer Admin-Bereich). if (!user && ctx.url.pathname.startsWith('/api/') && !ctx.url.pathname.startsWith('/api/admin')) { user = await tokenUser(ctx.request.headers.get('authorization')); diff --git a/src/pages/api/admin/users.ts b/src/pages/api/admin/users.ts index ecb3b9f..ccf3742 100644 --- a/src/pages/api/admin/users.ts +++ b/src/pages/api/admin/users.ts @@ -1,6 +1,6 @@ import type { APIRoute } from 'astro'; import { one, query } from '../../../lib/db'; -import { hashPassword } from '../../../lib/auth'; +import { hashPassword, bumpSessionEpoch } from '../../../lib/auth'; export const prerender = false; const json = (b: unknown, s = 200) => @@ -16,6 +16,8 @@ export const POST: APIRoute = async ({ request }) => { const b = await request.json(); if (b.id && b.password) { await query('UPDATE users SET password_hash=$2 WHERE id=$1', [b.id, await hashPassword(b.password)]); + // Neues Passwort heißt: alte Sitzungen gelten nicht mehr. + await bumpSessionEpoch(b.id); return json({ ok: true }); } if (b.id && 'private_forced' in b) { @@ -24,6 +26,9 @@ export const POST: APIRoute = async ({ request }) => { } if (b.id && b.role) { await query(`UPDATE users SET role=$2 WHERE id=$1`, [b.id, b.role === 'admin' ? 'admin' : 'user']); + // Rolle geändert → Sitzung entwerten, sonst behielte ein herabgestufter + // Admin seine Rechte bis zum Ablauf des Cookies. + await bumpSessionEpoch(b.id); return json({ ok: true }); } if (!b.username || !b.password) return json({ error: 'username + password nötig.' }, 400); diff --git a/src/pages/api/auth/login.ts b/src/pages/api/auth/login.ts index 2ea72d2..8de0c76 100644 --- a/src/pages/api/auth/login.ts +++ b/src/pages/api/auth/login.ts @@ -1,10 +1,14 @@ import type { APIRoute } from 'astro'; import { login, makeSessionCookie, rateLimited, noteFailure, clearFailures } from '../../../lib/auth'; +import { echteIp, zuOft } from '../../../lib/limit'; export const prerender = false; export const POST: APIRoute = async ({ request, clientAddress }) => { - const ip = clientAddress || 'unknown'; + // Nicht `clientAddress` direkt: Das ist der **erste** Wert aus + // X-Forwarded-For und damit vom Aufrufer selbst gesetzt — die Sperre nach + // fünf Fehlversuchen ließe sich sonst mit einem wechselnden Header umgehen. + const ip = echteIp(request, clientAddress); if (rateLimited(ip)) { return new Response(JSON.stringify({ error: 'Zu viele Versuche. Bitte in 15 Minuten erneut.' }), { status: 429, headers: { 'Content-Type': 'application/json' } }); @@ -19,15 +23,22 @@ export const POST: APIRoute = async ({ request, clientAddress }) => { username = String(f.get('username') || ''); password = String(f.get('password') || ''); } - const user = await login(username.trim(), password); - if (!user) { + // Zusätzlich je Benutzername zählen — sonst hilft die IP-Sperre nichts, wenn + // der Angreifer aus vielen Netzen kommt. + if (zuOft('login-user:' + username.trim().toLowerCase(), 10, 15 * 60_000)) { + return new Response(JSON.stringify({ error: 'Zu viele Versuche. Bitte in 15 Minuten erneut.' }), + { status: 429, headers: { 'Content-Type': 'application/json' } }); + } + + const angemeldet = await login(username.trim(), password); + if (!angemeldet) { noteFailure(ip); return new Response(JSON.stringify({ error: 'Benutzername oder Passwort falsch.' }), { status: 401, headers: { 'Content-Type': 'application/json' } }); } clearFailures(ip); - return new Response(JSON.stringify({ ok: true, role: user.role }), { + return new Response(JSON.stringify({ ok: true, role: angemeldet.user.role }), { status: 200, - headers: { 'Content-Type': 'application/json', 'Set-Cookie': makeSessionCookie(user) }, + headers: { 'Content-Type': 'application/json', 'Set-Cookie': makeSessionCookie(angemeldet.user, angemeldet.epoch) }, }); }; diff --git a/src/pages/api/auth/reset.ts b/src/pages/api/auth/reset.ts index 13163f0..832933b 100644 --- a/src/pages/api/auth/reset.ts +++ b/src/pages/api/auth/reset.ts @@ -1,8 +1,9 @@ import type { APIRoute } from 'astro'; import { randomBytes, createHash } from 'node:crypto'; import { one, query } from '../../../lib/db'; -import { hashPassword } from '../../../lib/auth'; -import { sendMail, mailLayout, mailReady, istMail } from '../../../lib/mail'; +import { hashPassword, bumpSessionEpoch } from '../../../lib/auth'; +import { sendMail, mailLayout, mailReady, istMail, esc } from '../../../lib/mail'; +import { zuOft, echteIp } from '../../../lib/limit'; export const prerender = false; const json = (b: unknown, s = 200) => @@ -12,17 +13,6 @@ const GUELTIG_MINUTEN = 60; const hashToken = (t: string) => createHash('sha256').update(t).digest('hex'); const basis = () => (process.env.PUBLIC_BASE_URL || '').replace(/\/+$/, ''); -// Ein Zähler je Adresse, damit niemand über diese Route Mails verschicken kann. -const versuche = new Map(); -function zuOft(schluessel: string, max = 5, fensterMs = 15 * 60_000): boolean { - const jetzt = Date.now(); - const e = versuche.get(schluessel); - if (!e || e.bis < jetzt) { versuche.set(schluessel, { n: 1, bis: jetzt + fensterMs }); return false; } - e.n++; - if (versuche.size > 5000) versuche.clear(); // simpler Schutz gegen Zuwachs - return e.n > max; -} - /** * Rücksetz-Link anfordern (`{ email }`) oder neues Passwort setzen * (`{ token, password }`). @@ -31,6 +21,7 @@ function zuOft(schluessel: string, max = 5, fensterMs = 15 * 60_000): boolean { * Adresse gibt. Sonst ließe sich hier ablesen, wer ein Konto hat. */ export const POST: APIRoute = async ({ request, clientAddress }) => { + const ip = echteIp(request, clientAddress); const b = await request.json().catch(() => ({})); // --- Neues Passwort setzen ------------------------------------------------- @@ -38,7 +29,7 @@ export const POST: APIRoute = async ({ request, clientAddress }) => { const token = String(b.token); const pw = String(b.password || ''); if (pw.length < 10) return json({ error: 'Das Passwort muss mindestens 10 Zeichen haben.' }, 400); - if (zuOft('einloesen:' + clientAddress, 20)) return json({ error: 'Zu viele Versuche. Bitte später erneut.' }, 429); + if (zuOft('reset-einloesen:' + ip, 20, 15 * 60_000)) return json({ error: 'Zu viele Versuche. Bitte später erneut.' }, 429); const r = await one<{ user_id: string }>( `SELECT user_id FROM password_resets @@ -48,13 +39,22 @@ export const POST: APIRoute = async ({ request, clientAddress }) => { await query('UPDATE users SET password_hash=$2 WHERE id=$1', [r.user_id, await hashPassword(pw)]); // Einmalnutzung, und alle anderen offenen Links desselben Kontos verfallen mit. await query('UPDATE password_resets SET used_at = now() WHERE user_id=$1 AND used_at IS NULL', [r.user_id]); + // Entscheidend: **alle bestehenden Sitzungen entwerten**. Wer das Passwort + // zurücksetzt, tut das meist, weil jemand anderes hineingekommen ist — ohne + // diesen Schritt bliebe dessen Cookie bis zu 30 Tage gültig und der ganze + // Vorgang wäre wirkungslos. + await bumpSessionEpoch(r.user_id); return json({ ok: true }); } // --- Rücksetz-Link anfordern ---------------------------------------------- const email = String(b.email || '').trim().toLowerCase(); if (!istMail(email)) return json({ error: 'Bitte eine gültige E-Mail-Adresse eingeben.' }, 400); - if (zuOft('anfordern:' + clientAddress)) return json({ ok: true }); // still schlucken + // Zwei Grenzen: je Absender **und** je Zieladresse. Ohne die zweite ließe sich + // ein bestimmtes Postfach von wechselnden Adressen aus zumüllen — auf Kosten + // des Zustellrufs der eigenen Domain. + if (zuOft('reset-ip:' + ip, 5, 15 * 60_000)) return json({ ok: true }); + if (zuOft('reset-mail:' + email, 3, 60 * 60_000)) return json({ ok: true }); if (!(await mailReady())) return json({ error: 'Auf diesem Server ist kein E-Mail-Versand eingerichtet.' }, 503); const u = await one<{ id: string; display_name: string | null; username: string }>( @@ -69,14 +69,17 @@ export const POST: APIRoute = async ({ request, clientAddress }) => { const url = `${basis()}/passwort?token=${token}`; const name = u.display_name || u.username; - await sendMail({ + // Nicht abwarten: Ein SMTP-Versand dauert Hunderte Millisekunden. Wartet die + // Antwort darauf, verrät allein die Dauer, ob es das Konto gibt — und die + // gleichlautende Antwort unten wäre wertlos. + void sendMail({ to: email, subject: 'Passwort für Klarbild zurücksetzen', text: `Hallo ${name},\n\nüber den folgenden Link kannst du dir ein neues Passwort für Klarbild setzen. ` + `Der Link gilt ${GUELTIG_MINUTEN} Minuten und lässt sich nur einmal verwenden.\n\n${url}\n\n` + `Wenn du das nicht angefordert hast, kannst du diese Nachricht ignorieren — es ändert sich dann nichts.`, html: mailLayout('Neues Passwort setzen', [ - `Hallo ${name},`, + `Hallo ${esc(name)},`, `über den Knopf unten setzt du dir ein neues Passwort für Klarbild. Der Link gilt ${GUELTIG_MINUTEN} Minuten und lässt sich nur einmal verwenden.`, `Wenn du das nicht angefordert hast, kannst du diese Nachricht ignorieren — es ändert sich dann nichts.`, ], { text: 'Neues Passwort setzen', url }), diff --git a/src/pages/api/me.ts b/src/pages/api/me.ts index 78cdd9d..561822c 100644 --- a/src/pages/api/me.ts +++ b/src/pages/api/me.ts @@ -39,7 +39,15 @@ export const PATCH: APIRoute = async ({ request, locals }) => { 'SELECT id FROM users WHERE lower(email)=$1 AND id<>$2', [roh, locals.user.uid]); if (belegt) return json({ error: 'Diese Adresse gehört schon zu einem anderen Konto.' }, 409); } - await query('UPDATE users SET email=$2 WHERE id=$1', [locals.user.uid, roh || null]); + try { + await query('UPDATE users SET email=$2 WHERE id=$1', [locals.user.uid, roh || null]); + } catch (e: any) { + // Zwei gleichzeitige Anfragen können an der Prüfung oben vorbeikommen — + // der eindeutige Index fängt es ab. Dann dieselbe verständliche Auskunft + // wie oben statt einer HTML-Fehlerseite. + if (e?.code === '23505') return json({ error: 'Diese Adresse gehört schon zu einem anderen Konto.' }, 409); + throw e; + } } if ('notify_jobs' in b) { diff --git a/src/pages/api/s/[slug]/file.ts b/src/pages/api/s/[slug]/file.ts index 7152622..8f62160 100644 --- a/src/pages/api/s/[slug]/file.ts +++ b/src/pages/api/s/[slug]/file.ts @@ -2,7 +2,7 @@ import type { APIRoute } from 'astro'; import sharp from 'sharp'; import { one } from '../../../../lib/db'; import { getObject } from '../../../../lib/storage'; -import { resolveShare, shareHasItem, trackShare } from '../../../../lib/shares'; +import { resolveShare, shareHasItem, trackShare, nachweisGilt, nachweisAusCookie } from '../../../../lib/shares'; export const prerender = false; @@ -15,7 +15,15 @@ export const prerender = false; */ export const GET: APIRoute = async ({ params, url, request, clientAddress }) => { const q = url.searchParams; - const { status, share } = await resolveShare(params.slug!, q.get('pw')); + // Kein `?pw=` mehr: Bei geschützten Links zählt allein der Nachweis, den die + // Seite nach der einmaligen Prüfung als Cookie gesetzt hat. Sonst liefe für + // jedes einzelne Bild erneut argon2 (130 ms, 64 MiB) — bei einer Galerie mit + // 40 Bildern reicht das, um den Prozess in die Speichergrenze zu treiben. + let { status, share } = await resolveShare(params.slug!, null); + if (status === 'passwort' && share + && nachweisGilt(share.id, nachweisAusCookie(request.headers.get('cookie'), params.slug!))) { + status = 'ok'; + } if (status !== 'ok' || !share) { return new Response( status === 'abgelaufen' ? 'Dieser Link ist abgelaufen.' @@ -50,13 +58,23 @@ export const GET: APIRoute = async ({ params, url, request, clientAddress }) => // Ansichtsfassung: kleiner und schneller, vor allem am Handy. Der Download // liefert weiterhin die unveränderte Originaldatei in voller Auflösung. - if (!download && !wantThumb && !item.has_alpha) { + // + // Wichtig: Das galt vorher **nicht** für freigestellte Motive — die liefen + // ungefiltert durch, in voller Auflösung, mit Rechtsklick speicherbar. Damit + // war „Herunterladen aus" genau bei Stickern und Freistellern wirkungslos. + // Alphabilder werden deshalb als PNG verkleinert statt übersprungen. + if (!download && !wantThumb) { try { - buf = await sharp(buf, { failOn: 'none' }) - .resize({ width: 2000, height: 2000, fit: 'inside', withoutEnlargement: true }) - .jpeg({ quality: 88, mozjpeg: true }).toBuffer(); - contentType = 'image/jpeg'; - } catch { /* dann eben das Original */ } + const klein = sharp(buf, { failOn: 'none', limitInputPixels: 400_000_000 }) + .resize({ width: 2000, height: 2000, fit: 'inside', withoutEnlargement: true }); + if (item.has_alpha) { buf = await klein.png({ compressionLevel: 8 }).toBuffer(); contentType = 'image/png'; } + else { buf = await klein.jpeg({ quality: 88, mozjpeg: true }).toBuffer(); contentType = 'image/jpeg'; } + } catch (e: any) { + // Nur bei gesperrtem Download ist das Original tabu — dann lieber nichts + // ausliefern als versehentlich die volle Auflösung. + console.error('[share] Ansichtsfassung fehlgeschlagen:', e?.message || e); + if (!share.allow_download) return new Response('Vorschau nicht verfügbar.', { status: 503 }); + } } if (download) { @@ -75,7 +93,10 @@ export const GET: APIRoute = async ({ params, url, request, clientAddress }) => headers['Content-Disposition'] = `attachment; filename="${name}"`; } return new Response(new Uint8Array(buf), { headers }); - } catch { + } catch (e: any) { + // Ursache ins Log — sonst sucht der Betreiber bei einem Speicherausfall + // vergeblich, während alle Empfänger „Bild nicht mehr vorhanden" sehen. + console.error('[share] Auslieferung fehlgeschlagen', path, e?.message || e); return new Response('Bild nicht mehr vorhanden.', { status: 404 }); } }; diff --git a/src/pages/api/shares/index.ts b/src/pages/api/shares/index.ts index d1e72bc..877ed23 100644 --- a/src/pages/api/shares/index.ts +++ b/src/pages/api/shares/index.ts @@ -1,7 +1,9 @@ import type { APIRoute } from 'astro'; -import { one } from '../../../lib/db'; +import { query } from '../../../lib/db'; import { createShare, listShares, MAX_ITEMS } from '../../../lib/shares'; +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + export const prerender = false; const json = (b: unknown, s = 200) => new Response(JSON.stringify(b), { status: s, headers: { 'Content-Type': 'application/json' } }); @@ -25,19 +27,28 @@ export const POST: APIRoute = async ({ request, locals }) => { if (!ids.length) return json({ error: 'Keine Bilder ausgewählt.' }, 400); if (ids.length > MAX_ITEMS) return json({ error: `Höchstens ${MAX_ITEMS} Bilder je Link.` }, 422); + // Kennungen zuerst prüfen — eine ungültige würde Postgres sonst mit + // „invalid input syntax for type uuid" abbrechen lassen und der Aufrufer + // bekäme einen 500 statt einer klaren Auskunft. + if (ids.some((id) => !UUID.test(id))) return json({ error: 'Ungültige Bild-Kennung.' }, 400); + const istAdmin = locals.user.role === 'admin'; - const s = await one<{ library_visibility: string }>('SELECT library_visibility FROM settings WHERE id=1'); - for (const id of ids) { - const it = await one( - `SELECT i.id, i.status, i.result_path, j.created_by, j.private - FROM items i JOIN jobs j ON j.id = i.job_id WHERE i.id = $1`, [id]); - if (!it?.result_path || it.status !== 'done') return json({ error: 'Ein Bild ist nicht (mehr) verfügbar.' }, 404); - const eigen = it.created_by === locals.user.uid; - if (!istAdmin && !eigen && (it.private || s?.library_visibility !== 'shared')) - return json({ error: 'Kein Zugriff auf eines der Bilder.' }, 403); + // Eine Abfrage statt einer je Bild: bei 500 Bildern waren das 500 Rundreisen. + const gefunden = await query<{ id: string; status: string; result_path: string | null; created_by: string | null; private: boolean; library_visibility: string }>( + `SELECT i.id, i.status, i.result_path, j.created_by, j.private, + (SELECT library_visibility FROM settings WHERE id=1) AS library_visibility + FROM items i JOIN jobs j ON j.id = i.job_id + WHERE i.id = ANY($1::uuid[])`, [ids]); + if (gefunden.length !== ids.length) return json({ error: 'Ein Bild ist nicht (mehr) verfügbar.' }, 404); + + for (const it of gefunden) { + if (!it.result_path || it.status !== 'done') return json({ error: 'Ein Bild ist nicht (mehr) verfügbar.' }, 404); // Private Aufträge werden bewusst nie geteilt — sie sollen ja gerade // keine Spur hinterlassen. if (it.private) return json({ error: 'Bilder aus privaten Sitzungen lassen sich nicht teilen.' }, 403); + const eigen = it.created_by === locals.user.uid; + if (!istAdmin && !eigen && it.library_visibility !== 'shared') + return json({ error: 'Kein Zugriff auf eines der Bilder.' }, 403); } try { @@ -50,6 +61,10 @@ export const POST: APIRoute = async ({ request, locals }) => { }); return json({ share }); } catch (e: any) { - return json({ error: e?.message || 'Link konnte nicht angelegt werden.' }, 400); + // Eigene Meldungen (Gültigkeit, leere Auswahl) durchreichen, Datenbank- + // Interna nicht — die verraten Tabellen- und Spaltennamen. + const eigen = /Gültigkeit|ausgewählt|Bilder je Link/.test(String(e?.message || '')); + if (!eigen) console.error('[shares] Anlegen fehlgeschlagen:', e?.message || e); + return json({ error: eigen ? e.message : 'Link konnte nicht angelegt werden.' }, 400); } }; diff --git a/src/pages/api/shares/send.ts b/src/pages/api/shares/send.ts index 27504cd..3fd682e 100644 --- a/src/pages/api/shares/send.ts +++ b/src/pages/api/shares/send.ts @@ -1,6 +1,7 @@ import type { APIRoute } from 'astro'; import { one } from '../../../lib/db'; -import { sendMail, mailLayout, mailReady, istMail } from '../../../lib/mail'; +import { sendMail, mailLayout, mailReady, istMail, esc } from '../../../lib/mail'; +import { zuOft } from '../../../lib/limit'; export const prerender = false; const json = (b: unknown, s = 200) => @@ -8,6 +9,7 @@ const json = (b: unknown, s = 200) => const basis = () => (process.env.PUBLIC_BASE_URL || '').replace(/\/+$/, ''); const MAX_EMPFAENGER = 10; +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; /** * Einen Teilen-Link per E-Mail verschicken. @@ -28,6 +30,16 @@ export const POST: APIRoute = async ({ request, locals }) => { if (schlecht.length) return json({ error: `Keine gültige Adresse: ${schlecht[0]}` }, 400); if (!(await mailReady())) return json({ error: 'Es ist kein E-Mail-Versand eingerichtet (Admin → E-Mail).' }, 503); + // Ohne absolute Basisadresse stünde in der Mail ein relativer Pfad, den kein + // Postfach anklickbar macht — dann lieber gar nicht verschicken. + if (!/^https?:\/\//.test(basis())) + return json({ error: 'Die Basisadresse des Servers fehlt (PUBLIC_BASE_URL) — der Link in der Mail wäre unbrauchbar.' }, 503); + + // Je Absender begrenzen, nicht je IP: Wer angemeldet ist, ist identifiziert. + if (zuOft('mailversand:' + locals.user.uid, 50, 60 * 60_000)) + return json({ error: 'Für diese Stunde ist genug verschickt. Bitte später weitermachen.' }, 429); + + if (!UUID.test(String(b.shareId || ''))) return json({ error: 'Unbekannter Link.' }, 404); const istAdmin = locals.user.role === 'admin'; const share = await one( @@ -36,6 +48,10 @@ export const POST: APIRoute = async ({ request, locals }) => { [b.shareId, locals.user.uid, istAdmin]); if (!share) return json({ error: 'Unbekannter Link.' }, 404); if (share.revoked_at) return json({ error: 'Dieser Link ist deaktiviert — erst wieder freigeben.' }, 409); + // Auch der Ablauf zählt: Sonst bekommt die Oma eine Mail auf eine Seite, die + // ihr „Dieser Link ist abgelaufen" zeigt. + if (share.expires_at && new Date(share.expires_at).getTime() < Date.now()) + return json({ error: 'Dieser Link ist abgelaufen — erst die Gültigkeit verlängern.' }, 409); const url = `${basis()}/s/${share.slug}`; const absender = locals.user.name || locals.user.username || 'Jemand'; @@ -48,9 +64,14 @@ export const POST: APIRoute = async ({ request, locals }) => { ? 'Zum Öffnen brauchst du außerdem das Zugangswort — das bekommst du getrennt.' : ''; + // `mailLayout` setzt Absätze bewusst roh ins HTML, damit funktioniert — + // alles, was aus Nutzereingaben stammt, muss deshalb **hier** maskiert werden. + // Vorher war nur die Nachricht maskiert, der Titel nicht: Damit ließ sich über + // den eigenen, per SPF und DKIM signierten Mailserver ein fremder Link in + // vertrautem Gewand verschicken. const absaetze = [ - `${absender} hat dir etwas über Klarbild geschickt: ${titel}.`, - ...(nachricht ? [nachricht.replace(/&/g, '&').replace(/')] : []), + `${esc(absender)} hat dir etwas über Klarbild geschickt: ${esc(titel)}.`, + ...(nachricht ? [esc(nachricht).replace(/\n/g, '
')] : []), [gueltig, wort].filter(Boolean).join(' '), ]; diff --git a/src/pages/s/[slug].astro b/src/pages/s/[slug].astro index 54c61d3..257008d 100644 --- a/src/pages/s/[slug].astro +++ b/src/pages/s/[slug].astro @@ -5,11 +5,42 @@ // die Bilder sehen und herunterladen können, sonst nichts. Keine Navigation, // kein Zugang zur Bibliothek, kein Hinweis darauf, was es sonst noch gibt. import Base from '../../layouts/Base.astro'; -import { resolveShare, shareItems, trackShare } from '../../lib/shares'; +import { resolveShare, shareItems, trackShare, nachweisBauen, nachweisGilt, nachweisAusCookie, nachweisName } from '../../lib/shares'; +import { zuOft, echteIp } from '../../lib/limit'; const slug = Astro.params.slug!; -const pw = Astro.url.searchParams.get('pw'); -const { status, share } = await resolveShare(slug, pw); + +// Zugangswort kommt per POST (nicht als `?pw=` in der Adresse — das landete +// sonst im Verlauf, in Proxy-Protokollen und in jedem weitergeleiteten Link). +// Nach erfolgreicher Prüfung merkt sich ein kurzlebiges, auf diesen Link +// beschränktes Cookie das Ergebnis: `argon2.verify` kostet 130 ms und 64 MiB +// und lief sonst bei **jedem** Bildaufruf erneut. +let eingabe: string | null = null; +let falsch = false; +let gesperrt = false; +if (Astro.request.method === 'POST') { + // Durchprobieren begrenzen: Ein Zugangswort, das man am Telefon durchgibt, + // ist kurz — ohne Bremse wäre Raten aussichtsreich, und jede Prüfung kostet + // 130 ms und 64 MiB. + if (zuOft(`wort:${slug}:${echteIp(Astro.request, Astro.clientAddress)}`, 10, 15 * 60_000)) { + gesperrt = true; + } else { + const f = await Astro.request.formData().catch(() => null); + eingabe = String(f?.get('pw') || '') || null; + } +} +const cookieKopf = Astro.request.headers.get('cookie'); +let { status, share } = await resolveShare(slug, eingabe); + +// Bereits nachgewiesen? Dann ohne erneutes Hashen durchlassen. +if (status === 'passwort' && share && nachweisGilt(share.id, nachweisAusCookie(cookieKopf, slug))) { + status = 'ok'; +} +if (status === 'ok' && share?.password_hash && eingabe) { + Astro.response.headers.append('Set-Cookie', + `${nachweisName(slug)}=${nachweisBauen(share.id)}; HttpOnly; Secure; SameSite=Lax; Path=/; Max-Age=21600`); +} +if (status === 'passwort' && eingabe) falsch = true; let bilder: Awaited> = []; if (status === 'ok' && share) { @@ -22,8 +53,9 @@ Astro.response.headers.set('X-Robots-Tag', 'noindex, nofollow, noarchive, noimag if (status !== 'ok') Astro.response.status = status === 'nicht-gefunden' ? 404 : status === 'passwort' ? 401 : 410; const titel = share?.title || (bilder.length > 1 ? 'Bilder' : 'Bild'); -const dl = (id: string) => `/api/s/${slug}/file?item=${id}&download=1${pw ? `&pw=${encodeURIComponent(pw)}` : ''}`; -const gross = (id: string) => `/api/s/${slug}/file?item=${id}${pw ? `&pw=${encodeURIComponent(pw)}` : ''}`; +const dl = (id: string) => `/api/s/${slug}/file?item=${id}&download=1`; +const gross = (id: string) => `/api/s/${slug}/file?item=${id}`; +const klein = (id: string) => `/api/s/${slug}/file?item=${id}&thumb=1`; const meldung: Record = { 'nicht-gefunden': { h: 'Diesen Link gibt es nicht', t: 'Vielleicht hat sich beim Kopieren ein Zeichen verloren. Frag am besten noch einmal nach.' }, 'abgelaufen': { h: 'Dieser Link ist abgelaufen', t: 'Teilen-Links gelten nur eine begrenzte Zeit. Wer ihn dir geschickt hat, kann einen neuen erzeugen.' }, @@ -50,7 +82,11 @@ const meldung: Record = { {bilder.map((b) => (
- {b.filename + {/* In der Übersicht das Vorschaubild — sonst rechnet der Server + für jede Kachel ein Vollbild neu. Das große Bild kommt erst + beim Antippen. */} + 1 ? klein(b.id) : gross(b.id)} + alt={b.filename || 'Bild'} loading="lazy" />
{b.output_px?.replace('x', ' × ') || ''} @@ -67,16 +103,23 @@ const meldung: Record = { Am Handy: Bild antippen, dann lange darauf drücken und „Zu Fotos hinzufügen" wählen. {share.allow_download && <> Über Herunterladen kommt die Datei in voller Auflösung.}

+

+ Wer dir diesen Link geschickt hat, sieht, wann und mit welcher Art Gerät er geöffnet + wurde — nicht, wer das war. Deine Adresse wird nur verschlüsselt abgelegt und nach + 90 Tagen gelöscht. +

) : status === 'passwort' ? (

Dieser Link ist mit einem Wort geschützt

Gib das Zugangswort ein, das du zusammen mit dem Link bekommen hast.

-
+
- {pw &&

Das war leider nicht richtig.

} + {gesperrt + ?

Zu viele Versuche. Bitte in einer Viertelstunde noch einmal.

+ : falsch &&

Das war leider nicht richtig.

}
) : (
@@ -105,6 +148,7 @@ const meldung: Record = { .leer { color: var(--soft); } .hinweis { margin-top: 28px; padding-top: 18px; border-top: 1px solid var(--line); font-size: 13px; color: var(--ink-soft); line-height: 1.6; max-width: 62ch; } + .datenschutz { margin: 10px 0 0; font-size: 12px; color: var(--soft); line-height: 1.6; max-width: 62ch; } .tor { max-width: 460px; margin: 8vh auto 0; text-align: center; } .tor h1 { font-size: 1.4rem; margin: 0 0 12px; } diff --git a/tests/mail.test.ts b/tests/mail.test.ts new file mode 100644 index 0000000..9d92bbe --- /dev/null +++ b/tests/mail.test.ts @@ -0,0 +1,66 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { istMail, mailLayout, esc } from '../src/lib/mail.ts'; + +test('Gültige Adressen werden erkannt', () => { + for (const gut of ['mail@tillheidrich.de', 'a.b+c@sub.beispiel.co.uk', 'x@y.de', 'Oma_1949@web.de']) { + assert.ok(istMail(gut), `fälschlich abgelehnt: ${gut}`); + } +}); + +test('Ungültige Adressen werden abgelehnt — auch die mit angehängtem Zeilenumbruch', () => { + const schlecht = [ + '', ' ', 'keine-adresse', 'a@b', '@b.de', 'a@.de', 'a b@c.de', 'a@b .de', + 'a@b.de\nBcc: opfer@example.com', // Kopfzeilen-Einschleusung + 'a@b.de\r\nSubject: x', + 'a@b.de\n', // JS-„$" passt auch vor dem Zeilenende + 'a'.repeat(250) + '@b.de', // zu lang + ]; + for (const s of schlecht) assert.ok(!istMail(s), `durchgerutscht: ${JSON.stringify(s)}`); +}); + +test('esc entschärft alles, was HTML zerlegen könnte', () => { + assert.equal(esc('x'), '<b>x</b>'); + assert.equal(esc('a & b'), 'a & b'); + assert.equal(esc('sagte "hallo"'), 'sagte "hallo"'); + // Reihenfolge: & zuerst, sonst würde < zu &lt; + assert.equal(esc('<&>'), '<&>'); +}); + +test('Die Überschrift der Mail wird immer maskiert', () => { + const html = mailLayout('', ['Text']); + assert.ok(!html.includes('' }); + assert.ok(!html.includes('