Hacker-Software/opencrm
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Pentest 56.1/56.2/56.3/56.4/56.5: Ownership-Checks + InvoiceType-Validierung

Browse Source 
56.1 HIGH (IDOR auf Upload-Endpoints):
- /upload/bank-cards/:id (POST/DELETE): canAccessBankCard +
  Existenz-Check, multer-Datei wird bei Reject sauber aufgeräumt.
- /upload/documents/:id (POST/DELETE): canAccessIdentityDocument
  + Existenz-Check + Cleanup.
- /upload/customers/:id/{business-registration,commercial-register,
  privacy-policy} (POST/DELETE): canAccessCustomer + Cleanup.
- /upload/invoices/:id (POST/DELETE): canAccessContract über
  Invoice→Contract-Resolve + Cleanup.

56.2 HIGH (IDOR + Consent-Eskalation bei privacy-policy):
- Vor dem upsert auf alle 4 CustomerConsent-Einträge (=GRANTED)
  läuft jetzt canAccessCustomer. Portal-Vertreter ohne Vollmacht
  oder Mitarbeiter mit anderer Customer-Beschränkung kommen
  damit nicht mehr durch.

56.3 LATENT (updateContract / deleteContract):
- Defense-in-Depth: canAccessContract jetzt explizit im Controller,
  nicht nur über die Route-Permission.

56.4 MEDIUM (invoiceType ungeprüft in addInvoiceByContract):
- Neuer assertValidInvoiceType-Helper mit Whitelist
  ['INTERIM','FINAL','NOT_AVAILABLE'] in addInvoice,
  updateInvoice und addInvoiceByContract. updateInvoice nur
  bei explizit gesetztem Wert; addInvoiceByContract zusätzlich
  die fehlende Required-Field-Validierung ergänzt.

56.5 LOW (GDPR-Löschanfragen ohne Ownership-Check):
- POST /api/gdpr/deletions liest customerId jetzt aus dem Body
  (Route hat kein :id-Segment), validiert auf positive Zahl und
  ruft canAccessCustomer auf, bevor die Löschanfrage erstellt wird.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
duffyduck
2026-06-01 21:01:06 +02:00
parent 72de2f00f3
commit a023e96012
4 changed files with 140 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/routes/upload.routes.ts
+91 -6
View File
@@ -6,7 +6,31 @@ 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';
import {
canAccessContract,
canAccessCustomer,
canAccessBankCard,
canAccessIdentityDocument,
} from '../utils/accessControl.js';
// Pentest 56.1 (HIGH, 2026-06-01): Upload-Endpoints prüften nur die
// Permission, nicht ob die Ziel-Resource zum Caller passt. Helper-Funktion
// für saubere 404-Antwort + Datei-Cleanup, wenn die Resource nicht
// existiert. Anschließend laufen die `canAccess*`-Checks (Portal-User
// werden dort auf ihre eigenen Kunden eingeschränkt; Staff bekommen
// volle Sicht – konsistent mit der bestehenden Access-Control-Logik).
async function resolveInvoiceContractId(invoiceId: number): Promise<number | null> {
const invoice = await prisma.invoice.findUnique({
where: { id: invoiceId },
select: { contractId: true, energyContractDetails: { select: { contractId: true } } },
});
return invoice?.contractId ?? invoice?.energyContractDetails?.contractId ?? null;
}
function cleanupFile(filePath?: string) {
if (!filePath) return;
try { fs.unlinkSync(filePath); } catch { /* ignore */ }
}
const router = Router();
@@ -164,9 +188,20 @@ router.post(
}
const bankCardId = parseInt(req.params.id);
// Pentest 56.1: Existenz- und Ownership-Check VOR DB-Update.
const card = await prisma.bankCard.findUnique({ where: { id: bankCardId } });
if (!card) {
cleanupFile(req.file.path);
res.status(404).json({ success: false, error: 'Bankkarte nicht gefunden' });
return;
}
if (!(await canAccessBankCard(req, res, bankCardId))) {
cleanupFile(req.file.path);
return;
}
const relativePath = `/uploads/bank-cards/${req.file.filename}`;
// Bankkarte in der DB aktualisieren
await prisma.bankCard.update({
where: { id: bankCardId },
data: { documentPath: relativePath },
@@ -183,6 +218,7 @@ router.post(
});
} catch (error) {
console.error('Upload error:', error);
cleanupFile(req.file?.path);
res.status(500).json({ success: false, error: 'Upload fehlgeschlagen' });
}
}
@@ -204,9 +240,20 @@ router.post(
}
const documentId = parseInt(req.params.id);
// Pentest 56.1: Existenz- und Ownership-Check VOR DB-Update.
const doc = await prisma.identityDocument.findUnique({ where: { id: documentId } });
if (!doc) {
cleanupFile(req.file.path);
res.status(404).json({ success: false, error: 'Ausweis nicht gefunden' });
return;
}
if (!(await canAccessIdentityDocument(req, res, documentId))) {
cleanupFile(req.file.path);
return;
}
const relativePath = `/uploads/documents/${req.file.filename}`;
// Ausweis in der DB aktualisieren
await prisma.identityDocument.update({
where: { id: documentId },
data: { documentPath: relativePath },
@@ -223,6 +270,7 @@ router.post(
});
} catch (error) {
console.error('Upload error:', error);
cleanupFile(req.file?.path);
res.status(500).json({ success: false, error: 'Upload fehlgeschlagen' });
}
}
@@ -246,6 +294,8 @@ router.delete(
res.status(404).json({ success: false, error: 'Bankkarte nicht gefunden' });
return;
}
// Pentest 56.1: Ownership-Check.
if (!(await canAccessBankCard(req, res, bankCardId))) return;
if (!bankCard.documentPath) {
res.status(400).json({ success: false, error: 'Kein Dokument vorhanden' });
@@ -290,6 +340,8 @@ router.delete(
res.status(404).json({ success: false, error: 'Ausweis nicht gefunden' });
return;
}
// Pentest 56.1: Ownership-Check.
if (!(await canAccessIdentityDocument(req, res, documentId))) return;
if (!document.documentPath) {
res.status(400).json({ success: false, error: 'Kein Dokument vorhanden' });
@@ -334,6 +386,11 @@ router.post(
}
const customerId = parseInt(req.params.id);
// Pentest 56.1: Ownership-Check.
if (!(await canAccessCustomer(req, res, customerId))) {
cleanupFile(req.file.path);
return;
}
const relativePath = `/uploads/business-registrations/${req.file.filename}`;
// Alte Datei löschen falls vorhanden
@@ -383,6 +440,11 @@ router.post(
}
const customerId = parseInt(req.params.id);
// Pentest 56.1: Ownership-Check.
if (!(await canAccessCustomer(req, res, customerId))) {
cleanupFile(req.file.path);
return;
}
const relativePath = `/uploads/commercial-registers/${req.file.filename}`;
// Alte Datei löschen falls vorhanden
@@ -424,6 +486,8 @@ router.delete(
async (req: AuthRequest, res: Response) => {
try {
const customerId = parseInt(req.params.id);
// Pentest 56.1: Ownership-Check.
if (!(await canAccessCustomer(req, res, customerId))) return;
const customer = await prisma.customer.findUnique({ where: { id: customerId } });
if (!customer) {
@@ -464,6 +528,8 @@ router.delete(
async (req: AuthRequest, res: Response) => {
try {
const customerId = parseInt(req.params.id);
// Pentest 56.1: Ownership-Check.
if (!(await canAccessCustomer(req, res, customerId))) return;
const customer = await prisma.customer.findUnique({ where: { id: customerId } });
if (!customer) {
@@ -514,6 +580,14 @@ router.post(
}
const customerId = parseInt(req.params.id);
// Pentest 56.2 (HIGH): Ownership-Check VOR Consent-Massen-Update.
// Ohne diese Prüfung konnte jeder Caller mit customers:update für
// jede beliebige customerId ALLE Einwilligungen auf GRANTED setzen
// (DSGVO-Eskalation).
if (!(await canAccessCustomer(req, res, customerId))) {
cleanupFile(req.file.path);
return;
}
const relativePath = `/uploads/privacy-policies/${req.file.filename}`;
// Alte Datei löschen falls vorhanden
@@ -574,6 +648,8 @@ router.delete(
async (req: AuthRequest, res: Response) => {
try {
const customerId = parseInt(req.params.id);
// Pentest 56.1: Ownership-Check.
if (!(await canAccessCustomer(req, res, customerId))) return;
const customer = await prisma.customer.findUnique({ where: { id: customerId } });
if (!customer) {
@@ -846,14 +922,20 @@ router.post(
}
const invoiceId = parseInt(req.params.id);
const relativePath = `/uploads/invoices/${req.file.filename}`;
// Alte Datei löschen falls vorhanden
// Pentest 56.1: Existenz- und Ownership-Check VOR DB-Update.
const invoice = await prisma.invoice.findUnique({ where: { id: invoiceId } });
if (!invoice) {
cleanupFile(req.file.path);
res.status(404).json({ success: false, error: 'Rechnung nicht gefunden' });
return;
}
const invoiceContractId = await resolveInvoiceContractId(invoiceId);
if (invoiceContractId == null || !(await canAccessContract(req, res, invoiceContractId))) {
cleanupFile(req.file.path);
return;
}
const relativePath = `/uploads/invoices/${req.file.filename}`;
if (invoice.documentPath) {
const oldPath = path.join(process.cwd(), invoice.documentPath);
@@ -898,6 +980,9 @@ router.delete(
res.status(404).json({ success: false, error: 'Rechnung nicht gefunden' });
return;
}
// Pentest 56.1: Ownership-Check vor Delete.
const invoiceContractId = await resolveInvoiceContractId(invoiceId);
if (invoiceContractId == null || !(await canAccessContract(req, res, invoiceContractId))) return;
if (!invoice.documentPath) {
res.status(400).json({ success: false, error: 'Kein Dokument vorhanden' });