Pentest 2026-05-20 LOW/INFO Sammelfix

27.1 Path-Traversal-Strings in DB: - cleanupConsents validierte documentPath zuvor nur per stripHtml, ließ "../../../etc/passwd" durch. Neuer isValidDocumentPath-Check akzeptiert nur "/uploads/<safe>", alles andere → NULL. - cleanupDocumentPaths scannt fünf weitere Tabellen (BankCard, IdentityDocument, Invoice, RepresentativeAuthorization nullable; ContractDocument NOT NULL → nur Report). Orphaned User: - reportOrphanedUsers warnt beim Container-Start vor User ohne Rollenzuordnung (im Permission-System unsichtbar). Löschen nicht automatisch wegen False-Positive-Risiko. Seed-PW-Policy: - generateInitialPassword() nutzte Math.random() (vorhersagbar). Jetzt crypto.randomInt() für Pick + Fisher-Yates-Shuffle. PUT /users/:id mit permissions / password: - Vorher silent-drop durch Whitelist + HTTP 200, Caller glaubte faelschlich, Werte waeren uebernommen. Jetzt HTTP 400 mit konkreter Hilfe-Message. /api/health ohne Auth: - Pentest-Befund INFO: bewusst so, Container-Healthcheck und Reverse-Proxy pingen ohne Bearer-Token. Antwort liefert nur {status,timestamp} – keine Version, kein DB-Status, kein Info-Leak. Comment im Code dokumentiert die Entscheidung. Live-verifiziert auf dev: alle fuenf Findings durchgetestet, jeweils mit dirty Input → erwartete Sanitization/Antwort. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-20 07:49:06 +02:00
parent adc3b70492
commit 8e48d3b432
5 changed files with 184 additions and 13 deletions
@@ -92,11 +92,26 @@ const ALLOWED_CONSENT_SOURCES = new Set([
  'crm-backend',
 ]);

+// Legitimer documentPath: relativer Pfad unter uploads/, keine ".."-Segmente.
+// Schreibend werden Pfade ausschließlich vom multer-Upload erzeugt
+// (server-kontrollierter Dateiname), bestehende Pentest-Hinterlassenschaften
+// wie "../../../etc/passwd" oder "javascript:alert(1)" müssen aus der DB
+// raus (Pentest 2026-05-20 LOW 27.1).
+function isValidDocumentPath(v: string | null | undefined): boolean {
+  if (!v) return true; // null/leer ist OK
+  if (v.includes('..')) return false;
+  if (/(?:javascript|data|vbscript)\s*:/i.test(v)) return false;
+  if (/<[a-z!\/]/i.test(v)) return false; // HTML im Pfad
+  // erlaubt: "uploads/...", "/uploads/..."; keine Kontrollzeichen
+  return /^\/?uploads\/[A-Za-z0-9._\-\/]+$/.test(v);
+}
+
 async function cleanupConsents() {
  // version + documentPath: HTML strippen (waren ohne Validierung).
  // source: Whitelist erzwingen.
+  // documentPath zusätzlich gegen Pfad-Traversal absichern (27.1).
  let versionStripped = 0;
-  let pathStripped = 0;
+  let pathNulled = 0;
  let sourceFixed = 0;
  const consents = await prisma.customerConsent.findMany({
    select: { id: true, source: true, documentPath: true, version: true },
@@ -107,9 +122,11 @@ async function cleanupConsents() {
      data.version = stripHtmlString(c.version);
      versionStripped++;
    }
-    if (c.documentPath && c.documentPath !== stripHtmlString(c.documentPath)) {
-      data.documentPath = stripHtmlString(c.documentPath);
-      pathStripped++;
+    if (c.documentPath && !isValidDocumentPath(c.documentPath)) {
+      // ".../etc/passwd", "<script>", "javascript:..." etc. → NULL.
+      // Legitime Uploads bleiben unberührt (siehe isValidDocumentPath).
+      data.documentPath = null;
+      pathNulled++;
    }
    if (c.source && !ALLOWED_CONSENT_SOURCES.has(c.source)) {
      data.source = 'unknown';
@@ -121,10 +138,100 @@ async function cleanupConsents() {
  }
  console.log(
    `  → Consent bereinigt: version-stripped=${versionStripped}, ` +
-    `documentPath-stripped=${pathStripped}, source-whitelist=${sourceFixed}`,
+    `documentPath-genullt=${pathNulled}, source-whitelist=${sourceFixed}`,
  );
 }

+// documentPath in den weiteren Tabellen prüfen. Schreibend wird er
+// server-seitig vom multer-Upload erzeugt – falls dort doch mal ein
+// dreckiger Wert reingerutscht ist (z.B. aus einem importierten Backup
+// vor unseren Sanitization-Fixes), nullen wir ihn hier raus.
+// ContractDocument hat documentPath NOT NULL → wir berichten dort nur,
+// löschen aber nicht (Records müssten manuell angeschaut werden).
+async function cleanupDocumentPaths() {
+  const findings: { table: string; id: number; value: string }[] = [];
+
+  const optional: Array<{
+    label: string;
+    fetch: () => Promise<{ id: number; documentPath: string | null }[]>;
+    update: (id: number) => Promise<unknown>;
+  }> = [
+    {
+      label: 'BankCard',
+      fetch: () => prisma.bankCard.findMany({ select: { id: true, documentPath: true } }),
+      update: (id) => prisma.bankCard.update({ where: { id }, data: { documentPath: null } }),
+    },
+    {
+      label: 'IdentityDocument',
+      fetch: () => prisma.identityDocument.findMany({ select: { id: true, documentPath: true } }),
+      update: (id) => prisma.identityDocument.update({ where: { id }, data: { documentPath: null } }),
+    },
+    {
+      label: 'Invoice',
+      fetch: () => prisma.invoice.findMany({ select: { id: true, documentPath: true } }),
+      update: (id) => prisma.invoice.update({ where: { id }, data: { documentPath: null } }),
+    },
+    {
+      label: 'RepresentativeAuthorization',
+      fetch: () => prisma.representativeAuthorization.findMany({
+        select: { id: true, documentPath: true },
+      }),
+      update: (id) => prisma.representativeAuthorization.update({
+        where: { id }, data: { documentPath: null },
+      }),
+    },
+  ];
+
+  let nulled = 0;
+  for (const t of optional) {
+    const rows = await t.fetch();
+    for (const r of rows) {
+      if (r.documentPath && !isValidDocumentPath(r.documentPath)) {
+        findings.push({ table: t.label, id: r.id, value: r.documentPath.slice(0, 80) });
+        await t.update(r.id);
+        nulled++;
+      }
+    }
+  }
+
+  // ContractDocument: documentPath ist NOT NULL → wir berichten nur.
+  const contractDocs = await prisma.contractDocument.findMany({
+    select: { id: true, documentPath: true },
+  });
+  let contractDocsDirty = 0;
+  for (const d of contractDocs) {
+    if (!isValidDocumentPath(d.documentPath)) {
+      findings.push({ table: 'ContractDocument', id: d.id, value: d.documentPath.slice(0, 80) });
+      contractDocsDirty++;
+    }
+  }
+
+  console.log(`  → documentPath bereinigt: ${nulled} genullt, ${contractDocsDirty} ContractDocument-Records auffällig (NOT NULL, manuell prüfen)`);
+  for (const f of findings.slice(0, 10)) {
+    console.log(`     [${f.table}#${f.id}] "${f.value}"`);
+  }
+}
+
+async function reportOrphanedUsers() {
+  // User ohne jegliche Rollenzuordnung können sich zwar einloggen, sind aber
+  // im Permission-System unsichtbar. Meist Überrest von gescheiterten Seeds
+  // oder manuellen DB-Edits. Wir löschen NICHT (könnte legitime
+  // Spezial-User treffen) – nur warnen.
+  const orphans = await prisma.user.findMany({
+    where: { roles: { none: {} } },
+    select: { id: true, email: true, createdAt: true },
+  });
+  if (orphans.length === 0) {
+    console.log('  → Keine User ohne Rollenzuordnung.');
+    return;
+  }
+  console.log(`  ⚠️  ${orphans.length} User ohne Rollenzuordnung:`);
+  for (const u of orphans.slice(0, 10)) {
+    console.log(`     [User#${u.id}] ${u.email} (created ${u.createdAt.toISOString()})`);
+  }
+  console.log('     → Rolle zuweisen oder User löschen.');
+}
+
 async function cleanupAppSettings() {
  const settings = await prisma.appSetting.findMany();
  const removed: string[] = [];
@@ -228,6 +335,8 @@ async function main() {
  await cleanupXss();
  await cleanupAppSettings();
  await cleanupConsents();
+  await cleanupDocumentPaths();
+  await reportOrphanedUsers();
  await findOrPurgePentestRecords();
  console.log('=== Fertig. ===');
 }
@@ -235,15 +235,17 @@ async function main() {
    const digits = '23456789';
    const special = '!@#$%&*+=?';
    const all = upper + lower + digits + special;
-    const pick = (s: string) => s[Math.floor(Math.random() * s.length)];
+    // Kryptografisch sichere Auswahl – Math.random() ist vorhersagbar
+    // und reicht für ein Initial-Admin-Passwort nicht (Pentest 2026-05-20).
+    const pick = (s: string) => s[crypto.randomInt(0, s.length)];
    // mind. einen aus jeder Klasse + Rest zufällig
    const chars = [pick(upper), pick(lower), pick(digits), pick(special)];
    // 28 Zeichen → Komplexität + komfortable Marge über dem 25-Zeichen-
    // Mitarbeiter-Schwellwert (Pentest Runde 13).
    for (let i = chars.length; i < 28; i++) chars.push(pick(all));
-    // Fisher-Yates Shuffle (sonst stehen die garantierten Klassen-Zeichen am Anfang)
+    // Fisher-Yates Shuffle mit kryptografisch starkem Random.
    for (let i = chars.length - 1; i > 0; i--) {
-      const j = Math.floor(Math.random() * (i + 1));
+      const j = crypto.randomInt(0, i + 1);
      [chars[i], chars[j]] = [chars[j], chars[i]];
    }
    return chars.join('');
@@ -81,10 +81,32 @@ export async function createUser(req: Request, res: Response): Promise<void> {
 export async function updateUser(req: Request, res: Response): Promise<void> {
  try {
    const userId = parseInt(req.params.id);
+    // `permissions` und `password` darf der generische Update nicht
+    // entgegennehmen. Vorher landeten sie auf dem Floor (Whitelist-Drop),
+    // der Caller bekam aber 200 zurück und glaubte fälschlich, die Werte
+    // wären übernommen worden. Stattdessen sofort 400, damit Tooling /
+    // Client den Fehler sieht. (Pentest 2026-05-20)
+    //   - permissions kommen aus Rollen (PUT roleIds bzw. die DSGVO-/
+    //     Developer-Checkboxen) und können nicht direkt am User hängen.
+    //   - password wird über POST /users/:id/password gesetzt
+    //     (eigene Komplexitäts-Validierung + Audit-Trail).
+    const body = req.body || {};
+    const forbidden = ['permissions', 'password', 'passwordHash'];
+    const offending = forbidden.filter((k) => k in body);
+    if (offending.length > 0) {
+      res.status(400).json({
+        success: false,
+        error: `Felder nicht erlaubt: ${offending.join(', ')}. ` +
+          (offending.includes('permissions')
+            ? 'Permissions werden über roleIds / hasGdprAccess / hasDeveloperAccess gesteuert. '
+            : '') +
+          (offending.includes('password') || offending.includes('passwordHash')
+            ? `Passwort über POST /users/${userId}/password setzen.`
+            : ''),
+      } as ApiResponse);
+      return;
+    }
    // Whitelist: nur erlaubte Felder aus req.body übernehmen (Mass-Assignment-Schutz)
-    // password ist NICHT in der Whitelist – generisches Update darf kein
-    // Passwort setzen. Dafür gibt es POST /users/:id/password mit eigenem
-    // Audit-Eintrag (Pentest Runde 12, MITTEL).
    const data = pickUserUpdate(req.body);

    // Vorherigen Stand laden für Audit – inkl. Rollen, damit hasGdprAccess /
@@ -367,8 +367,13 @@ app.use('/api/birthdays', birthdayRoutes);
 app.use('/api/factory-defaults', factoryDefaultsRoutes);
 app.use('/api/monitoring', monitoringRoutes);

-// Health check
-app.get('/api/health', (req, res) => {
+// Health check – BEWUSST ohne Auth (Container-Healthcheck und Reverse-Proxy
+// pingen das ohne Bearer-Token). Antwort enthält absichtlich nur statisch
+// "ok" + Timestamp, keine Version, kein DB-Status, kein Hostname – damit
+// auch unauth Caller keine internen Infos einsammeln können. Pentest
+// 2026-05-20 (INFO): kein Auth → akzeptiert, Antwort liefert nichts
+// Sensibles.
+app.get('/api/health', (_req, res) => {
  res.json({ status: 'ok', timestamp: new Date().toISOString() });
 });

@@ -120,6 +120,39 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
  - **Live-verifiziert**: 4867 Datensätze + 1 Datei in 13.2s
    wiederhergestellt, Log-Modal zeigt den vollständigen Verlauf.

+- [x] **🧹 Pentest 2026-05-20 LOW/INFO-Sammelfix**
+  - **27.1 Path-Traversal-Strings in DB**: `cleanupConsents` validierte
+    `documentPath` zuvor nur per stripHtml, was `../../../etc/passwd`
+    durchließ (kein File-Read, aber dreckige Datenbasis). Neuer
+    `isValidDocumentPath`-Check akzeptiert nur `/?uploads/<safe>`,
+    alles andere wird auf `NULL` gesetzt.
+  - Generischer `cleanupDocumentPaths`-Pass über die fünf weiteren
+    Tabellen mit `documentPath` (BankCard, IdentityDocument, Invoice,
+    RepresentativeAuthorization als nullable; ContractDocument
+    NOT NULL → wird nur berichtet, manuell entscheiden).
+  - **Orphaned User**: Neuer Report-Step `reportOrphanedUsers` warnt
+    beim Container-Start vor User ohne Rollenzuordnung (= im
+    Permission-System unsichtbar). Löschen tut das Skript nicht
+    (false-positive-Risiko bei legitimen Spezial-Usern).
+  - **Seed-PW-Policy**: `generateInitialPassword()` nutzte
+    `Math.random()` (vorhersagbar). Jetzt `crypto.randomInt()` für
+    Pick + Shuffle, 28 Zeichen aus 4 Klassen.
+  - **`PUT /users/:id` mit `permissions` / `password`**: vorher
+    silent-drop durch Whitelist + HTTP 200. Jetzt explizit HTTP 400
+    mit Hinweis auf den dedizierten `/password`-Endpoint bzw. die
+    Role-Steuerung. Gleicher Pattern wie `PUT /portal` für password.
+  - **`/api/health` ohne Auth**: BEWUSST so – Container-Healthcheck +
+    Reverse-Proxy pingen ohne Bearer-Token. Antwort liefert nur
+    `{status,timestamp}` – keine Version, kein DB-Status, kein
+    Hostname → kein Info-Leak. Kommentar im Code dokumentiert die
+    Entscheidung.
+  - **Live-verifiziert** auf dev:
+    - CustomerConsent.documentPath=`../../../etc/passwd` → NULL
+    - PUT /users mit `permissions` → 400 mit klarer Message
+    - PUT /users mit `password` → 400 mit Hinweis auf /password
+    - Orphan-User angelegt → vom Cleanup-Lauf gemeldet
+    - `crypto.randomInt`-Pfad rauscht durch ohne Fehler
+
 - [x] **🛡️ Pentest 2026-05-20 MEDIUM+LOW: Consent + URI-Sanitization**
  - **MEDIUM Consent-Mass-Assignment**: PUT `/api/gdpr/customer/:id/consents/:type`
    nahm `source`, `documentPath`, `version` ungefiltert aus dem Body