Security-Hardening Runde 5: Hack-Das-Ding (DSGVO-GAU + Timing + XSS)
Live-Pentest gegen Dev-Server + 3 parallele Audit-Agents. 🚨 CRITICAL: /api/uploads/* war ohne Auth erreichbar - express.static('/api/uploads', ...) → jeder konnte mit ratbarer URL sensible PDFs (Kündigungsbestätigungen, Ausweise, Bankkarten, Vollmachten) ziehen. Live-verifiziert: 23-KB-PDF eines echten Kunden ohne Login geladen. - Fix: authenticate-Middleware vor static-Handler (req.query.token unterstützung war schon da, jetzt aktiv genutzt). - Frontend: utils/fileUrl.ts hängt JWT als ?token=... an. 24 direkte /api${...Path}-URLs in 5 Dateien per Skript migriert (CustomerDetail, ContractDetail, InvoicesSection, PdfTemplates, GDPRDashboard). 🚨 HIGH: Login-Timing User-Enumeration - bcrypt.compare wurde nur bei existierenden Usern ausgeführt → 110ms vs 10ms Differenz, Email-Enumeration trivial messbar. - Fix: Dummy-bcrypt-compare bei invalid user (Cost 12). Plus Lazy- Rehash bei erfolgreichem Login: alte Cost-10-Hashes (z.B. admin aus Installation) werden auf BCRYPT_COST upgraded, damit Dummy- und Echt-Hash-Cost zusammenpassen. - Live-verifiziert nach Admin-Rehash: 422ms (invalid) vs 423ms (valid) – Side-Channel dicht. 🚨 HIGH: XSS via Privacy-Policy/Imprint-HTML - 4 Frontend-Seiten renderten Backend-HTML ohne DOMPurify (PortalPrivacy, ConsentPage, PortalWebsitePrivacy, PortalImprint). Admin-eingegebene <script>-Tags wären bei jedem Portal-Kunden- Besuch ausgeführt worden – auch auf der öffentlichen Consent-Seite. - Fix: DOMPurify.sanitize mit strikter FORBID_TAGS/ATTR Config. 🛡 HIGH: IDOR-Härtung an Upload-/Document-Endpoints - canAccessContract jetzt in: uploadContractDocument, deleteContractDocument, handleContractDocumentUpload (Kündigungs- Letter+Confirmation), handleContractDocumentDelete, saveAttachmentAsContractDocument. - Defense-in-Depth: aktuell durch requirePermission abgesichert, schützt auch gegen künftige Staff-Scoping-Rollen. Offen für v1.1: - Per-File-Ownership-Check für /api/uploads (Kontroll-Lookup welche Ressource zur Datei gehört) - TipTap-Link-Tool javascript:-Protokoll blockieren - Prisma-Error-Messages in Admin-Endpoints generisch sanitisieren Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1926,6 +1926,9 @@ export async function saveAttachmentAsContractDocument(req: Request, res: Respon
|
||||
return;
|
||||
}
|
||||
|
||||
// Ownership-Check (Portal-Kunde darf nur auf eigenen/vertretenen Vertrag)
|
||||
if (!(await canAccessContract(req as AuthRequest, res, contract.id))) return;
|
||||
|
||||
// Für gesendete E-Mails: Prüfen ob UID vorhanden
|
||||
if (email.folder === 'SENT' && email.uid === 0) {
|
||||
res.status(400).json({
|
||||
|
||||
@@ -462,6 +462,7 @@ export async function getContractDocuments(req: AuthRequest, res: Response): Pro
|
||||
export async function uploadContractDocument(req: AuthRequest, res: Response): Promise<void> {
|
||||
try {
|
||||
const contractId = parseInt(req.params.id);
|
||||
if (!(await canAccessContract(req, res, contractId))) return;
|
||||
const { documentType, notes, deliveryDate } = req.body;
|
||||
|
||||
if (!req.file) {
|
||||
@@ -511,6 +512,7 @@ export async function deleteContractDocument(req: AuthRequest, res: Response): P
|
||||
try {
|
||||
const documentId = parseInt(req.params.documentId);
|
||||
const contractId = parseInt(req.params.id);
|
||||
if (!(await canAccessContract(req, res, contractId))) return;
|
||||
|
||||
const doc = await prisma.contractDocument.findUnique({ where: { id: documentId } });
|
||||
if (!doc || doc.contractId !== contractId) {
|
||||
|
||||
@@ -38,6 +38,7 @@ import { startBirthdayScheduler } from './services/birthdayScheduler.service.js'
|
||||
import { startContractStatusScheduler } from './services/contractStatusScheduler.service.js';
|
||||
import { auditContextMiddleware } from './middleware/auditContext.js';
|
||||
import { auditMiddleware } from './middleware/audit.js';
|
||||
import { authenticate } from './middleware/auth.js';
|
||||
|
||||
dotenv.config();
|
||||
|
||||
@@ -95,8 +96,12 @@ app.use(express.json({ limit: '5mb' }));
|
||||
app.use(auditContextMiddleware);
|
||||
app.use(auditMiddleware);
|
||||
|
||||
// Statische Dateien für Uploads
|
||||
app.use('/api/uploads', express.static(path.join(process.cwd(), 'uploads')));
|
||||
// Statische Dateien für Uploads – NUR für authentifizierte User.
|
||||
// authenticate-Middleware unterstützt ?token=... Query-Parameter für direkte
|
||||
// <a href>-Downloads, bei denen der Browser keinen Authorization-Header sendet.
|
||||
// Ohne diesen Schutz könnte jeder per Datei-Name-Enumeration sensible PDFs
|
||||
// (Ausweise, Kündigungsbestätigungen, Bankkarten) abrufen – DSGVO-GAU.
|
||||
app.use('/api/uploads', authenticate as any, express.static(path.join(process.cwd(), 'uploads')));
|
||||
|
||||
// Öffentliche Routes (OHNE Authentifizierung)
|
||||
app.use('/api/public/consent', consentPublicRoutes);
|
||||
|
||||
@@ -6,6 +6,7 @@ import prisma from '../lib/prisma.js';
|
||||
import { authenticate, requirePermission } from '../middleware/auth.js';
|
||||
import { AuthRequest } from '../types/index.js';
|
||||
import { logChange } from '../services/audit.service.js';
|
||||
import { canAccessContract } from '../utils/accessControl.js';
|
||||
|
||||
const router = Router();
|
||||
|
||||
@@ -546,6 +547,7 @@ async function handleContractDocumentUpload(
|
||||
}
|
||||
|
||||
const contractId = parseInt(req.params.id);
|
||||
if (!(await canAccessContract(req, res, contractId))) return;
|
||||
const relativePath = `/uploads/${subDir}/${req.file.filename}`;
|
||||
|
||||
// Alte Datei löschen falls vorhanden
|
||||
@@ -631,6 +633,7 @@ async function handleContractDocumentDelete(
|
||||
) {
|
||||
try {
|
||||
const contractId = parseInt(req.params.id);
|
||||
if (!(await canAccessContract(req, res, contractId))) return;
|
||||
|
||||
const contract = await prisma.contract.findUnique({ where: { id: contractId } });
|
||||
if (!contract) {
|
||||
|
||||
@@ -11,6 +11,40 @@ import { getSystemEmailCredentials } from './emailProvider/emailProviderService.
|
||||
// Bestehende Hashes mit Faktor 10 bleiben gültig (bcrypt kodiert den Faktor im Hash).
|
||||
const BCRYPT_COST = 12;
|
||||
|
||||
// Dummy-Hash mit Cost 12 für Timing-Attack-Schutz: bei nicht-existierendem User
|
||||
// führen wir trotzdem ein bcrypt.compare() durch, damit die Antwortzeit nicht
|
||||
// verrät, ob die E-Mail existiert. Konstanter Hash hat keine Bedeutung außer
|
||||
// dem Timing-Angleich.
|
||||
const DUMMY_BCRYPT_HASH = '$2a$12$CwTycUXWue0Thq9StjUM0uJ8gQKwqKjq8lZ3TZ9qg8aJ0A9hPn4Wy';
|
||||
|
||||
/**
|
||||
* Upgrade eines bestehenden Passwort-Hashes auf aktuellen BCRYPT_COST.
|
||||
* Wird nach erfolgreichem Login aufgerufen. Alte User (z.B. admin mit Cost 10
|
||||
* aus der Installation) werden so lazy auf Cost 12 migriert – damit sich die
|
||||
* Antwortzeit beim Login der Dummy-Zeit bei ungültigen Usern angleicht.
|
||||
*/
|
||||
async function maybeUpgradePasswordHash(
|
||||
table: 'user' | 'customer',
|
||||
id: number,
|
||||
plaintextPassword: string,
|
||||
currentHash: string,
|
||||
): Promise<void> {
|
||||
const match = currentHash.match(/^\$2[aby]\$(\d+)\$/);
|
||||
const currentCost = match ? parseInt(match[1], 10) : 0;
|
||||
if (currentCost === BCRYPT_COST) return;
|
||||
try {
|
||||
const newHash = await bcrypt.hash(plaintextPassword, BCRYPT_COST);
|
||||
if (table === 'user') {
|
||||
await prisma.user.update({ where: { id }, data: { password: newHash } });
|
||||
} else {
|
||||
await prisma.customer.update({ where: { id }, data: { portalPasswordHash: newHash } });
|
||||
}
|
||||
} catch (err) {
|
||||
// Nicht kritisch – Login war erfolgreich, Rehash kann beim nächsten Login nachgeholt werden
|
||||
console.warn('[maybeUpgradePasswordHash] Fehler beim Rehash:', err);
|
||||
}
|
||||
}
|
||||
|
||||
// Mitarbeiter-Login
|
||||
export async function login(email: string, password: string) {
|
||||
const user = await prisma.user.findUnique({
|
||||
@@ -33,6 +67,9 @@ export async function login(email: string, password: string) {
|
||||
});
|
||||
|
||||
if (!user || !user.isActive) {
|
||||
// Timing-Attack-Schutz: Dummy-bcrypt-compare damit die Antwortzeit bei
|
||||
// nicht-existierendem/deaktiviertem User der eines gültigen Users entspricht.
|
||||
await bcrypt.compare(password, DUMMY_BCRYPT_HASH);
|
||||
throw new Error('Ungültige Anmeldedaten');
|
||||
}
|
||||
|
||||
@@ -41,6 +78,10 @@ export async function login(email: string, password: string) {
|
||||
throw new Error('Ungültige Anmeldedaten');
|
||||
}
|
||||
|
||||
// Lazy-Upgrade: ältere Cost-10-Hashes auf aktuellen BCRYPT_COST rehashen.
|
||||
// Async, nicht blockierend für die Response.
|
||||
maybeUpgradePasswordHash('user', user.id, password, user.password).catch(() => {});
|
||||
|
||||
// Collect all permissions from all roles
|
||||
const permissions = new Set<string>();
|
||||
for (const userRole of user.roles) {
|
||||
@@ -107,6 +148,8 @@ export async function customerLogin(email: string, password: string) {
|
||||
|
||||
if (!customer || !customer.portalEnabled || !customer.portalPasswordHash) {
|
||||
console.log('[CustomerLogin] Abbruch: Kunde nicht gefunden oder Portal nicht aktiviert');
|
||||
// Timing-Attack-Schutz (siehe login())
|
||||
await bcrypt.compare(password, DUMMY_BCRYPT_HASH);
|
||||
throw new Error('Ungültige Anmeldedaten');
|
||||
}
|
||||
|
||||
@@ -117,6 +160,9 @@ export async function customerLogin(email: string, password: string) {
|
||||
throw new Error('Ungültige Anmeldedaten');
|
||||
}
|
||||
|
||||
// Lazy-Upgrade analog zu Mitarbeiter-Login
|
||||
maybeUpgradePasswordHash('customer', customer.id, password, customer.portalPasswordHash).catch(() => {});
|
||||
|
||||
// Letzte Anmeldung aktualisieren
|
||||
await prisma.customer.update({
|
||||
where: { id: customer.id },
|
||||
|
||||
@@ -141,6 +141,36 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
|
||||
- Provider/Tariff-GETs: `requirePermission('providers:read')` (Portal-Kunden sehen Provider-Liste nicht mehr)
|
||||
- SMTP-Header-Injection: zentrale CRLF-Validierung in `smtpService.sendEmail` (schützt alle Caller)
|
||||
- bcrypt cost 10 → 12 (OWASP 2026)
|
||||
- **Runde 5 – Hack-Das-Ding-Audit (Live-Pentest + 3 parallele Audit-Agents):**
|
||||
- 🚨 **`/api/uploads/*` war OHNE AUTH erreichbar** (DSGVO-GAU!) – jetzt hinter
|
||||
`authenticate`. Direkte <a href>-Links nutzen `?token=...` Query-Parameter,
|
||||
unterstützt von auth-Middleware. Frontend-Helper `fileUrl(path)` hängt
|
||||
Token automatisch an, 24 URLs migriert (CustomerDetail, ContractDetail,
|
||||
InvoicesSection, PdfTemplates, GDPRDashboard).
|
||||
- **Login-Timing-Side-Channel**: Bei ungültigem User fehlte `bcrypt.compare`
|
||||
→ 110ms vs 10ms, User-Enumeration trivial. Jetzt Dummy-bcrypt-compare
|
||||
(Cost 12) bei invalid user + Lazy-Rehash alter Cost-10-Hashes beim Login.
|
||||
Live-verifiziert: 422ms vs 425ms – Timing-Angriff dicht.
|
||||
- **XSS via Privacy Policy / Imprint**: 4 Frontend-Seiten renderten
|
||||
Backend-HTML ohne DOMPurify (`PortalPrivacy`, `ConsentPage`,
|
||||
`PortalWebsitePrivacy`, `PortalImprint`). Admin-eingegebene
|
||||
`<script>`-Tags wären bei jedem Portal-Kunden-Besuch ausgeführt worden.
|
||||
Jetzt mit strikter Sanitize-Config (FORBID_TAGS/ATTR).
|
||||
- **IDOR-Härtung Upload/Delete/SaveAttachment**: `canAccessContract` jetzt
|
||||
in `uploadContractDocument`, `deleteContractDocument`, im generischen
|
||||
`handleContractDocumentUpload` (Kündigungsschreiben + -bestätigungen)
|
||||
und in `saveAttachmentAsContractDocument`. Defense-in-Depth, blockt
|
||||
auch bei künftigen Staff-Scoping-Rollen.
|
||||
- Global Error-Handler: `err.status` wird respektiert (413/400 statt 500).
|
||||
|
||||
**Offen für v1.1**:
|
||||
- Per-File-Ownership-Check bei `/api/uploads/*` (aktuell reicht
|
||||
Authentifizierung, kein Datei-spezifischer Owner-Check). Implementierung
|
||||
bräuchte dedizierten `GET /api/files/download?path=...`-Endpoint mit
|
||||
DB-Lookup, welche Ressource zur Datei gehört.
|
||||
- TipTap-Link-Tool: `javascript:`-Protokoll blockieren (Admin-only erreichbar,
|
||||
niedrig-Prio).
|
||||
|
||||
- **Runde 4 – Live-Tests gegen Dev-Server deckten 9 weitere IDORs auf:**
|
||||
- `getCustomer` + `getAddresses`/`getBankCards`/`getDocuments`/`getMeters`/`getRepresentatives`/`getPortalSettings` hatten NUR Daten-Sanitizer aber KEINEN `canAccessCustomer`-Check
|
||||
- `gdpr.getCustomerConsents` + `getAuthorizations` + `checkConsentStatus` ebenso ungeschützt
|
||||
|
||||
Reference in New Issue
Block a user