fix(security): Befunde aus dem Security-Audit beheben #7
No files matched your search
@@ -0,0 +1,13 @@
|
|||||||
|
import { PERMISSIONS_KEY } from './permissions.decorator';
|
||||||
|
import { Permission } from './permissions.enum';
|
||||||
|
import { ApiKeysController } from './api-keys.controller';
|
||||||
|
|
||||||
|
describe('ApiKeysController', () => {
|
||||||
|
it('verlangt MANAGE_SETTINGS – sonst kann sich jeder OIDC-Benutzer einen API-Key erzeugen', () => {
|
||||||
|
const required: unknown = Reflect.getMetadata(
|
||||||
|
PERMISSIONS_KEY,
|
||||||
|
ApiKeysController,
|
||||||
|
);
|
||||||
|
expect(required).toEqual([Permission.MANAGE_SETTINGS]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -9,9 +9,14 @@ import {
|
|||||||
} from '@nestjs/common';
|
} from '@nestjs/common';
|
||||||
import { ApiKeysService } from './api-keys.service';
|
import { ApiKeysService } from './api-keys.service';
|
||||||
import { JwtAuthGuard } from './jwt-auth.guard';
|
import { JwtAuthGuard } from './jwt-auth.guard';
|
||||||
|
import { RequirePermissions } from './permissions.decorator';
|
||||||
|
import { Permission } from './permissions.enum';
|
||||||
|
|
||||||
@Controller('api/api-keys')
|
@Controller('api/api-keys')
|
||||||
@UseGuards(JwtAuthGuard)
|
@UseGuards(JwtAuthGuard)
|
||||||
|
// Ein API-Key umgeht im PermissionsGuard jede Rechteprüfung – Anlegen und
|
||||||
|
// Verwalten darf daher nur, wer ohnehin alle Einstellungen verwalten darf.
|
||||||
|
@RequirePermissions(Permission.MANAGE_SETTINGS)
|
||||||
export class ApiKeysController {
|
export class ApiKeysController {
|
||||||
constructor(private readonly apiKeysService: ApiKeysService) {}
|
constructor(private readonly apiKeysService: ApiKeysService) {}
|
||||||
|
|
||||||
|
|||||||
@@ -27,6 +27,7 @@ import {
|
|||||||
kennzahlenAusPdf,
|
kennzahlenAusPdf,
|
||||||
type VergleichKennzahlen,
|
type VergleichKennzahlen,
|
||||||
} from '../paperless/vergleich-kennzahlen.util';
|
} from '../paperless/vergleich-kennzahlen.util';
|
||||||
|
import { parseSendEmailBody } from './send-email-body.util';
|
||||||
|
|
||||||
@Controller('api/inbox')
|
@Controller('api/inbox')
|
||||||
@RequirePermissions(Permission.VIEW_SCANNER)
|
@RequirePermissions(Permission.VIEW_SCANNER)
|
||||||
@@ -284,17 +285,10 @@ export class InboxController {
|
|||||||
@HttpCode(204)
|
@HttpCode(204)
|
||||||
async sendEmail(
|
async sendEmail(
|
||||||
@Param('id') id: string,
|
@Param('id') id: string,
|
||||||
@Body()
|
@Body() rawBody: unknown,
|
||||||
body: {
|
|
||||||
to: string;
|
|
||||||
subject: string;
|
|
||||||
body: string;
|
|
||||||
html?: string;
|
|
||||||
segments: { pages: number[]; filename: string }[];
|
|
||||||
sender?: string;
|
|
||||||
},
|
|
||||||
@Request() req: any,
|
@Request() req: any,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const body = parseSendEmailBody(rawBody);
|
||||||
const preferredUsername: string | null =
|
const preferredUsername: string | null =
|
||||||
req.user?.preferredUsername ?? null;
|
req.user?.preferredUsername ?? null;
|
||||||
const smtpOverride =
|
const smtpOverride =
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
import { BadRequestException } from '@nestjs/common';
|
||||||
|
import { parseSendEmailBody } from './send-email-body.util';
|
||||||
|
|
||||||
|
describe('parseSendEmailBody', () => {
|
||||||
|
const gueltig = {
|
||||||
|
to: 'a@example.org',
|
||||||
|
subject: 'Betreff',
|
||||||
|
body: 'Text',
|
||||||
|
html: '<p>Text</p>',
|
||||||
|
segments: [{ pages: [1, 2], filename: 'beleg' }],
|
||||||
|
sender: 'user',
|
||||||
|
};
|
||||||
|
|
||||||
|
it('übernimmt einen gültigen Body', () => {
|
||||||
|
expect(parseSendEmailBody(gueltig)).toEqual(gueltig);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('lässt html und sender weg, wenn sie fehlen', () => {
|
||||||
|
const ohne = { ...gueltig, html: undefined, sender: undefined };
|
||||||
|
expect(parseSendEmailBody(ohne)).toEqual({
|
||||||
|
to: gueltig.to,
|
||||||
|
subject: gueltig.subject,
|
||||||
|
body: gueltig.body,
|
||||||
|
segments: gueltig.segments,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
['html als Objekt mit path', { html: { path: '/proc/self/environ' } }],
|
||||||
|
['body als Objekt mit href', { body: { href: 'http://169.254.169.254/' } }],
|
||||||
|
['to als Objekt', { to: { address: 'a@example.org' } }],
|
||||||
|
['to als Array', { to: ['a@example.org'] }],
|
||||||
|
['subject fehlt', { subject: undefined }],
|
||||||
|
['segments kein Array', { segments: 'x' }],
|
||||||
|
['Seite keine Zahl', { segments: [{ pages: ['1'], filename: 'x' }] }],
|
||||||
|
[
|
||||||
|
'filename kein String',
|
||||||
|
{ segments: [{ pages: [1], filename: { path: '/etc/passwd' } }] },
|
||||||
|
],
|
||||||
|
['sender kein String', { sender: { x: 1 } }],
|
||||||
|
])('lehnt ab: %s', (_name, patch) => {
|
||||||
|
expect(() => parseSendEmailBody({ ...gueltig, ...patch })).toThrow(
|
||||||
|
BadRequestException,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('lehnt einen fehlenden Body ab', () => {
|
||||||
|
expect(() => parseSendEmailBody(undefined)).toThrow(BadRequestException);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,66 @@
|
|||||||
|
import { BadRequestException } from '@nestjs/common';
|
||||||
|
|
||||||
|
export interface SendEmailBody {
|
||||||
|
to: string;
|
||||||
|
subject: string;
|
||||||
|
body: string;
|
||||||
|
html?: string;
|
||||||
|
segments: { pages: number[]; filename: string }[];
|
||||||
|
sender?: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Prüft den Body von `POST inbox/:id/send-email` streng auf Strings bzw. Zahlen.
|
||||||
|
* Ohne diese Prüfung landen Objekte wie `{ path: '/proc/self/environ' }` direkt
|
||||||
|
* bei nodemailer, das sie als Datei- bzw. URL-Quelle interpretiert.
|
||||||
|
*/
|
||||||
|
export function parseSendEmailBody(raw: unknown): SendEmailBody {
|
||||||
|
if (!isObject(raw)) throw new BadRequestException('Ungültiger Request-Body');
|
||||||
|
const { to, subject, body, html, segments, sender } = raw;
|
||||||
|
|
||||||
|
for (const [name, wert] of [
|
||||||
|
['to', to],
|
||||||
|
['subject', subject],
|
||||||
|
['body', body],
|
||||||
|
] as const) {
|
||||||
|
if (typeof wert !== 'string') {
|
||||||
|
throw new BadRequestException(`"${name}" muss ein Text sein`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (html !== undefined && typeof html !== 'string') {
|
||||||
|
throw new BadRequestException('"html" muss ein Text sein');
|
||||||
|
}
|
||||||
|
if (sender !== undefined && typeof sender !== 'string') {
|
||||||
|
throw new BadRequestException('"sender" muss ein Text sein');
|
||||||
|
}
|
||||||
|
if (!Array.isArray(segments)) {
|
||||||
|
throw new BadRequestException('"segments" muss eine Liste sein');
|
||||||
|
}
|
||||||
|
const geprueft = segments.map((seg) => {
|
||||||
|
if (
|
||||||
|
!isObject(seg) ||
|
||||||
|
typeof seg.filename !== 'string' ||
|
||||||
|
!Array.isArray(seg.pages) ||
|
||||||
|
!seg.pages.every((p) => Number.isInteger(p))
|
||||||
|
) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
'Jedes Segment braucht "filename" (Text) und "pages" (ganze Zahlen)',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return { pages: seg.pages as number[], filename: seg.filename };
|
||||||
|
});
|
||||||
|
|
||||||
|
const ergebnis: SendEmailBody = {
|
||||||
|
to: to as string,
|
||||||
|
subject: subject as string,
|
||||||
|
body: body as string,
|
||||||
|
segments: geprueft,
|
||||||
|
};
|
||||||
|
if (html !== undefined) ergebnis.html = html;
|
||||||
|
if (sender !== undefined) ergebnis.sender = sender;
|
||||||
|
return ergebnis;
|
||||||
|
}
|
||||||
|
|
||||||
|
function isObject(v: unknown): v is Record<string, unknown> {
|
||||||
|
return typeof v === 'object' && v !== null && !Array.isArray(v);
|
||||||
|
}
|
||||||
@@ -0,0 +1,51 @@
|
|||||||
|
import { ConfigService } from '@nestjs/config';
|
||||||
|
import * as nodemailer from 'nodemailer';
|
||||||
|
import { MailService } from './mail.service';
|
||||||
|
|
||||||
|
jest.mock('nodemailer');
|
||||||
|
|
||||||
|
describe('MailService', () => {
|
||||||
|
const sendMail = jest.fn().mockResolvedValue({
|
||||||
|
messageId: 'x',
|
||||||
|
accepted: [],
|
||||||
|
rejected: [],
|
||||||
|
response: 'ok',
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
sendMail.mockClear();
|
||||||
|
(nodemailer.createTransport as jest.Mock).mockReturnValue({ sendMail });
|
||||||
|
});
|
||||||
|
|
||||||
|
const config = {
|
||||||
|
get: (_k: string, d?: unknown) => d,
|
||||||
|
} as unknown as ConfigService;
|
||||||
|
|
||||||
|
it.each([false, true])(
|
||||||
|
'verbietet nodemailer Datei- und URL-Zugriffe (smtpOverride=%s)',
|
||||||
|
async (override) => {
|
||||||
|
const service = new MailService(config);
|
||||||
|
await service.sendMail({
|
||||||
|
to: 'a@example.org',
|
||||||
|
subject: 's',
|
||||||
|
body: 'b',
|
||||||
|
smtpOverride: override
|
||||||
|
? {
|
||||||
|
host: 'h',
|
||||||
|
port: 25,
|
||||||
|
secure: false,
|
||||||
|
user: 'u',
|
||||||
|
pass: 'p',
|
||||||
|
from: 'f',
|
||||||
|
}
|
||||||
|
: undefined,
|
||||||
|
});
|
||||||
|
expect(sendMail).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
disableFileAccess: true,
|
||||||
|
disableUrlAccess: true,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
});
|
||||||
@@ -69,6 +69,10 @@ export class MailService {
|
|||||||
filename: a.filename,
|
filename: a.filename,
|
||||||
content: a.content,
|
content: a.content,
|
||||||
})),
|
})),
|
||||||
|
// Inhalte kommen teils vom Aufrufer: nodemailer darf nie Dateien vom
|
||||||
|
// Server lesen oder URLs abrufen (`{ path }` / `{ href }`).
|
||||||
|
disableFileAccess: true,
|
||||||
|
disableUrlAccess: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
this.logger.log(
|
this.logger.log(
|
||||||
|
|||||||
Reference in new issue
Block a user