Pentest R91: NaN-Bypass auf accountId-Query-Param

R91.1 LOW: parseInt('abc') = NaN → der Ternary gab NaN an den
Service, if (NaN) ist falsy → Postfach-Filter fiel weg. Portal-
User mit ungültigem accountId sah Mails aus allen Postfächern
des Kunden für seinen Vertrag (canAccessContract greift weiter,
kein Cross-Customer-Leak).

Fix: zentraler parsePositiveIntParam(), akzeptiert nur positive
Ganzzahlen aus Query-Strings. Eingesetzt auf allen 5 Endpunkten,
die accountId/contractId aus Query lesen – auch da, wo der
Pentester nicht getestet hat (Customer-Inbox, Trash-Count),
weil derselbe Pattern überall stand.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
2026-06-21 14:22:40 +02:00
parent 993f2d10f0
commit 18a2e1173b
2 changed files with 41 additions and 7 deletions
@@ -41,12 +41,31 @@ function parseDateParam(v: unknown): Date | undefined {
return isNaN(d.getTime()) ? undefined : d;
}
// Pentest 91.1 (LOW, 2026-06-21): `accountId=abc` (→ parseInt = NaN) und
// `accountId=` (leer string ist truthy nach `req.query.accountId ? …`)
// kamen vorher als `NaN` im Service an. `if (NaN)` ist falsy → der
// stressfreiEmailId-Filter wurde komplett übersprungen, und der Vertrag
// zeigte Mails aus ALLEN Postfächern.
//
// Helper akzeptiert nur positive Ganzzahlen; alles andere → undefined,
// das im Service den Filter wegfallen lässt. Wenn wir wollten dass
// invalide Werte 400 werfen statt silent ignoriert zu werden, müssten
// wir das pro Endpunkt entscheiden die Semantik „Filter weglassen"
// ist hier in Ordnung, weil der `customerId`-/`contractId`-Constraint
// im `where` weiterhin greift (kein Cross-Customer-Leak).
function parsePositiveIntParam(v: unknown): number | undefined {
if (typeof v !== 'string' || v.trim() === '') return undefined;
const n = parseInt(v, 10);
if (!Number.isFinite(n) || n < 1 || !Number.isInteger(n)) return undefined;
return n;
}
// E-Mails für einen Kunden abrufen
export async function getEmailsForCustomer(req: AuthRequest, res: Response): Promise<void> {
try {
const customerId = parseInt(req.params.customerId);
if (!(await canAccessCustomer(req, res, customerId))) return;
const stressfreiEmailId = req.query.accountId ? parseInt(req.query.accountId as string) : undefined;
const stressfreiEmailId = parsePositiveIntParam(req.query.accountId);
const folder = req.query.folder as string | undefined; // INBOX oder SENT
const limit = req.query.limit ? parseInt(req.query.limit as string) : 50;
const offset = req.query.offset ? parseInt(req.query.offset as string) : 0;
@@ -90,7 +109,7 @@ export async function getEmailsForContract(req: AuthRequest, res: Response): Pro
const contractId = parseInt(req.params.contractId);
if (!(await canAccessContract(req, res, contractId))) return;
const folder = req.query.folder as string | undefined; // INBOX oder SENT
const stressfreiEmailId = req.query.accountId ? parseInt(req.query.accountId as string) : undefined;
const stressfreiEmailId = parsePositiveIntParam(req.query.accountId);
const limit = req.query.limit ? parseInt(req.query.limit as string) : 50;
const offset = req.query.offset ? parseInt(req.query.offset as string) : 0;
@@ -248,7 +267,7 @@ export async function getContractFolderCounts(req: AuthRequest, res: Response):
try {
const contractId = parseInt(req.params.contractId);
if (!(await canAccessContract(req, res, contractId))) return;
const stressfreiEmailId = req.query.accountId ? parseInt(req.query.accountId as string) : undefined;
const stressfreiEmailId = parsePositiveIntParam(req.query.accountId);
const counts = await cachedEmailService.getFolderCountsForContract(contractId, stressfreiEmailId);
@@ -896,8 +915,8 @@ export async function getTrashEmails(req: AuthRequest, res: Response): Promise<v
try {
const customerId = parseInt(req.params.customerId);
if (!(await canAccessCustomer(req, res, customerId))) return;
const stressfreiEmailId = req.query.accountId ? parseInt(req.query.accountId as string) : undefined;
const contractId = req.query.contractId ? parseInt(req.query.contractId as string) : undefined;
const stressfreiEmailId = parsePositiveIntParam(req.query.accountId);
const contractId = parsePositiveIntParam(req.query.contractId);
const emails = await cachedEmailService.getTrashEmails(customerId, {
stressfreiEmailId,
@@ -919,8 +938,8 @@ export async function getTrashCount(req: AuthRequest, res: Response): Promise<vo
try {
const customerId = parseInt(req.params.customerId);
if (!(await canAccessCustomer(req, res, customerId))) return;
const stressfreiEmailId = req.query.accountId ? parseInt(req.query.accountId as string) : undefined;
const contractId = req.query.contractId ? parseInt(req.query.contractId as string) : undefined;
const stressfreiEmailId = parsePositiveIntParam(req.query.accountId);
const contractId = parsePositiveIntParam(req.query.contractId);
const count = await cachedEmailService.getTrashCount(customerId, {
stressfreiEmailId,
+15
View File
@@ -97,6 +97,21 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
## ✅ Erledigt
- [x] **🔒 Pentest R91 NaN-Bypass auf accountId-Query-Param**
- R91.1 (LOW): `accountId=abc``parseInt('abc')` = `NaN` → der
Ternary im Controller gab `NaN` an den Service, `if (NaN)` ist
falsy → der Postfach-Filter fiel weg. Folge: ein Portal-User mit
ungültigem `accountId` sah alle Mailbox-Mails für seinen Vertrag
statt nur die aus dem gewählten Postfach (kein Cross-Customer-
Leak — `canAccessContract` greift weiter).
- Fix: zentraler `parsePositiveIntParam()` im `cachedEmail.controller.ts`,
der nur positive Ganzzahlen aus dem Query-String akzeptiert und
alles andere zu `undefined` macht. Eingesetzt in allen 5
Endpunkten, die `accountId`/`contractId` aus Query nehmen
(Contract-Emails, Contract-Folder-Counts, Customer-Emails,
Trash, Trash-Count) auch da, wo der Pentester nicht getestet
hat, weil derselbe Pattern überall stand.
- [x] **🐞 E-Mail-Ansicht: Postfach-Filter griff in Trash/Sent nicht**
- Bug-Bericht 2026-06-21: im Vertrags-Tab (Gesendet/Gelöscht) und im
Kunden-Haupt-Postfach (Gelöscht) wurden E-Mails aus ALLEN Postfächern