mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
fix: stop buffering up to 100 MB of every unauthenticated request body before routing
This commit is contained in:
@@ -4,10 +4,12 @@ import { logger } from '@/lib/logger';
|
||||
import { getStalwartCredentials } from '@/lib/stalwart/credentials';
|
||||
import { fetchJmapServer } from '@/lib/stalwart/server-fetch';
|
||||
import { DisallowedUrlError } from '@/lib/security/url-guard';
|
||||
import { UpstreamBodyTooLargeError, readTextCapped } from '@/lib/security/capped-body';
|
||||
import { UpstreamBodyTooLargeError, readBodyCapped, readTextCapped } from '@/lib/security/capped-body';
|
||||
|
||||
/** A PROPFIND listing of a folder with many thousand entries still fits. */
|
||||
const MAX_PROPFIND_RESPONSE_BYTES = 50 * 1024 * 1024;
|
||||
/** PROPFIND and MKCALENDAR bodies are a few lines of XML; proxy.ts doesn't run here to cap them. */
|
||||
const MAX_XML_REQUEST_BYTES = 1024 * 1024;
|
||||
|
||||
const ALLOWED_METHODS = new Set(['PROPFIND', 'MKCOL', 'MKCALENDAR', 'GET', 'PUT', 'DELETE', 'MOVE', 'COPY']);
|
||||
|
||||
@@ -106,7 +108,8 @@ export async function POST(request: NextRequest) {
|
||||
// PROPFIND and MKCALENDAR bodies are small XML and are read fully.
|
||||
let body: ArrayBuffer | ReadableStream<Uint8Array> | null = null;
|
||||
if (method === 'PROPFIND' || method === 'MKCALENDAR') {
|
||||
body = await request.arrayBuffer();
|
||||
body = await readBodyCapped(request, MAX_XML_REQUEST_BYTES);
|
||||
if (!body) return NextResponse.json({ error: 'Request body too large' }, { status: 413 });
|
||||
} else if (method === 'PUT') {
|
||||
body = request.body;
|
||||
}
|
||||
|
||||
@@ -2,6 +2,14 @@ import { NextRequest, NextResponse } from 'next/server';
|
||||
import { logger } from '@/lib/logger';
|
||||
import { downloadFileBlob, getWopiFileNode, updateFileNodeBlob, uploadFileBlob } from '@/lib/wopi/files';
|
||||
import { wopiContext } from '@/lib/wopi/request';
|
||||
import { readBodyCapped } from '@/lib/security/capped-body';
|
||||
|
||||
/**
|
||||
* Largest document PutFile takes. proxy.ts doesn't run for this route, so
|
||||
* nothing else caps the copy held in memory; past it the save is refused, not
|
||||
* cut short. Stalwart's own upload limit (maxSizeUpload) is usually lower.
|
||||
*/
|
||||
const MAX_PUT_FILE_BYTES = 100 * 1024 * 1024;
|
||||
|
||||
/**
|
||||
* WOPI GetFile / PutFile (#425). Content flows purely over JMAP: GetFile
|
||||
@@ -111,7 +119,10 @@ export async function POST(
|
||||
}
|
||||
}
|
||||
|
||||
const body = await request.arrayBuffer();
|
||||
const body = await readBodyCapped(request, MAX_PUT_FILE_BYTES);
|
||||
if (!body) {
|
||||
return NextResponse.json({ error: 'Document too large' }, { status: 413 });
|
||||
}
|
||||
if (body.byteLength === 0) {
|
||||
return NextResponse.json({ error: 'Empty document rejected' }, { status: 409 });
|
||||
}
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*
|
||||
* Next copies the body of every request proxy.ts runs for, up to
|
||||
* proxyClientMaxBodySize, before the route sees it. That limit stays at the
|
||||
* 10 MB default; the upload routes that need more are left out of the
|
||||
* proxy's matcher. The matcher is read from proxy.ts the way `next build`
|
||||
* reads it (static extraction), and a path counts as proxied the way Next's
|
||||
* router decides it: when the raw or the decoded path matches.
|
||||
*/
|
||||
import path from 'node:path';
|
||||
import { describe, it, expect, vi } from 'vitest';
|
||||
import type { NextConfig } from 'next';
|
||||
import { loadBindings } from 'next/dist/build/swc';
|
||||
import { getPagesPageStaticInfo } from 'next/dist/build/analysis/get-page-static-info';
|
||||
import { getMiddlewareRouteMatcher } from 'next/dist/shared/lib/router/utils/middleware-route-matcher';
|
||||
import type { PAGE_TYPES } from 'next/dist/lib/page-types';
|
||||
|
||||
async function proxied(basePath = '') {
|
||||
await loadBindings();
|
||||
const info = await getPagesPageStaticInfo({
|
||||
pageFilePath: path.resolve(__dirname, '../../proxy.ts'),
|
||||
nextConfig: { basePath },
|
||||
isDev: false,
|
||||
page: '/proxy',
|
||||
pageType: 'pages' as PAGE_TYPES,
|
||||
});
|
||||
const matchers = info.middleware?.matchers;
|
||||
expect(matchers?.length).toBeGreaterThan(0);
|
||||
const match = getMiddlewareRouteMatcher(matchers!);
|
||||
const req = { headers: {} } as Parameters<typeof match>[1];
|
||||
return (p: string) => {
|
||||
let decoded = p;
|
||||
try { decoded = decodeURIComponent(p); } catch { /* as Next: keep it */ }
|
||||
return match(p, req, {}) || match(decoded, req, {});
|
||||
};
|
||||
}
|
||||
|
||||
describe('proxy matcher', () => {
|
||||
it('leaves out exactly the upload routes', async () => {
|
||||
const runs = await proxied();
|
||||
for (const p of ['/api/webdav', '/api/plugin-proxy', '/api/wopi/files/f123/contents', '/api/wopi/files/a%20b/contents']) {
|
||||
expect(runs(p), p).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('runs for every other path, including near misses and other spellings', async () => {
|
||||
const runs = await proxied();
|
||||
for (const p of [
|
||||
'/', '/en/mail', '/admin', '/setup', '/api/settings', '/api/auth/session', '/api/account/stalwart/jmap',
|
||||
'/api/webdav/', '/api/webdav/x', '/api/webdavx', '/api/plugin-proxy/x', '/api/plugin-proxy2',
|
||||
'/api/wopi/files/f123', '/api/wopi/files/f123/contents/x', '/api/wopi/launch', '/api/wopi/files//contents',
|
||||
'/api/%77ebdav', '/api/wopi/files/a%2Fb/contents', '/x/api/webdav', '/_next/static/chunk.js',
|
||||
]) expect(runs(p), p).toBe(true);
|
||||
});
|
||||
|
||||
it('applies under a base path', async () => {
|
||||
const runs = await proxied('/webmail');
|
||||
expect(runs('/webmail/api/webdav')).toBe(false);
|
||||
expect(runs('/webmail/api/settings')).toBe(true);
|
||||
});
|
||||
|
||||
it('keeps the body limit at the default and gives the left-out routes the proxy\'s API headers', async () => {
|
||||
vi.resetModules();
|
||||
const { default: nextConfig } = (await import('../../next.config')) as { default: NextConfig };
|
||||
expect(nextConfig.experimental?.proxyClientMaxBodySize).toBeUndefined();
|
||||
const rules = (await nextConfig.headers?.()) ?? [];
|
||||
for (const source of ['/api/webdav', '/api/plugin-proxy', '/api/wopi/files/:fileId/contents']) {
|
||||
const headers = rules.find((rule) => rule.source === source)?.headers ?? [];
|
||||
expect(headers, source).toEqual(expect.arrayContaining([
|
||||
{ key: 'X-Content-Type-Options', value: 'nosniff' },
|
||||
{ key: 'X-Frame-Options', value: 'DENY' },
|
||||
]));
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -114,6 +114,15 @@ describe('POST /api/webdav — proxying', () => {
|
||||
expect(res.headers!.get('Content-Type')).toBe('application/xml; charset=utf-8');
|
||||
});
|
||||
|
||||
it('PROPFIND refuses a request body over 1 MB without sending it on', async () => {
|
||||
const big = endlessResponse(50);
|
||||
const req = { ...makeReq({ 'X-WebDAV-Method': 'PROPFIND', 'X-WebDAV-Path': 'dir' }), body: big.response.body };
|
||||
const res = read(await POST(req as unknown as Parameters<typeof POST>[0]));
|
||||
expect(res.status).toBe(413);
|
||||
expect(big.pulled()).toBeLessThan(5);
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('PROPFIND answers 502 instead of buffering a listing that never ends', async () => {
|
||||
const { response, pulled } = endlessResponse(500, { status: 207 });
|
||||
fetchSpy.mockResolvedValueOnce(response);
|
||||
|
||||
@@ -44,6 +44,7 @@ import { fetchJmapServer } from '@/lib/stalwart/server-fetch';
|
||||
import { fetchJmapSession, postJmap } from '@/lib/stalwart/jmap-api';
|
||||
import { shareeSavesNeedRename, updateFileNodeBlob } from '@/lib/wopi/files';
|
||||
import { mintWopiToken, wopiDocumentId } from '@/lib/wopi/token';
|
||||
import { endlessResponse } from './helpers/endless-body';
|
||||
|
||||
process.env.ADMIN_STATE_DIR = mkdtempSync(path.join(tmpdir(), 'bw-wopi-files-'));
|
||||
|
||||
@@ -263,6 +264,22 @@ describe('WOPI PutFile for a Files node', () => {
|
||||
expect(setCalls()).toEqual([]);
|
||||
});
|
||||
|
||||
it('refuses a document over the size limit instead of saving part of it', async () => {
|
||||
const { data } = await launchFile({ fileId: 'f1' });
|
||||
const call = editorCall(data);
|
||||
const big = endlessResponse(200);
|
||||
const req = {
|
||||
...request(`/api/wopi/files/${call.documentId}/contents?${call.token}`, { headers: { 'X-WOPI-Override': 'PUT' } }),
|
||||
body: big.response.body,
|
||||
} as unknown as Parameters<typeof putFile>[0];
|
||||
|
||||
const res = await putFile(req, params(call.documentId));
|
||||
expect(res.status).toBe(413);
|
||||
expect(big.pulled()).toBeLessThan(110);
|
||||
expect(uploads()).toEqual([]);
|
||||
expect(setCalls()).toEqual([]);
|
||||
});
|
||||
|
||||
it('turns a refused update into a server error', async () => {
|
||||
const { data } = await launchFile({ fileId: 'f1' });
|
||||
(postJmap as Mock).mockImplementation(async (_url: string, _auth: string, body: string) => {
|
||||
|
||||
@@ -13,7 +13,7 @@ import { authHeaderFor, credentialOrigin, privateProxyOrigins, type CredentialIn
|
||||
import { getUserCredential, setUserCredentialSecret, userKey, type VaultUser } from './vault';
|
||||
import { declaredUserCredential } from './known-plugins';
|
||||
|
||||
/** Largest request body the proxy forwards (one upload chunk); see proxyClientMaxBodySize in next.config.ts. */
|
||||
/** Largest request body the proxy forwards (one upload chunk). */
|
||||
export const MAX_PROXY_REQUEST_BYTES = 100 * 1024 * 1024;
|
||||
/** Largest response body it passes back (a download). */
|
||||
export const MAX_PROXY_RESPONSE_BYTES = 2 * 1024 * 1024 * 1024;
|
||||
@@ -306,8 +306,8 @@ export async function proxyRequest(input: ProxyInput): Promise<Response> {
|
||||
const headers = forwardHeaders(input.headers, credential);
|
||||
// Read the whole body first and send it with a Content-Length. Streamed
|
||||
// (chunked) uploads reach some servers empty: Paperless (Django) answers
|
||||
// "No file was submitted". Next has buffered the body for proxy.ts
|
||||
// already, up to the same limit (see proxyClientMaxBodySize).
|
||||
// "No file was submitted". proxy.ts doesn't run for /api/plugin-proxy,
|
||||
// so this is the only copy, and the only limit.
|
||||
let body = input.body && method !== 'GET' && method !== 'HEAD'
|
||||
? await readBody(input.body, MAX_PROXY_REQUEST_BYTES)
|
||||
: null;
|
||||
|
||||
@@ -21,10 +21,10 @@ export class UpstreamBodyTooLargeError extends Error {
|
||||
|
||||
/**
|
||||
* The body as bytes, or null once it exceeds `maxBytes`, in which case the
|
||||
* upstream connection is aborted rather than drained.
|
||||
* upstream connection is aborted rather than drained. Takes a request too.
|
||||
*/
|
||||
export async function readBodyCapped(
|
||||
response: PublicFetchResponse | Response,
|
||||
response: PublicFetchResponse | Response | Request,
|
||||
maxBytes: number,
|
||||
controller?: AbortController,
|
||||
): Promise<ArrayBuffer | null> {
|
||||
|
||||
+13
-8
@@ -61,14 +61,10 @@ const nextConfig: NextConfig = {
|
||||
// it from node_modules at runtime instead of trying to bundle it. Used by
|
||||
// PLUGIN_DEV_DIR's on-the-fly bundler.
|
||||
serverExternalPackages: ["esbuild"],
|
||||
experimental: {
|
||||
// proxy.ts runs for every request, /api included, and Next buffers the
|
||||
// body for it up to this size and silently cuts off the rest. 10 MB (the
|
||||
// default) truncated WOPI saves and WebDAV uploads of bigger files, and
|
||||
// plugin uploads through /api/plugin-proxy. Keep MAX_PROXY_REQUEST_BYTES
|
||||
// in lib/plugin-secrets/proxy.ts in step with it.
|
||||
proxyClientMaxBodySize: "100mb",
|
||||
},
|
||||
// No proxyClientMaxBodySize: Next copies every body proxy.ts runs for up to
|
||||
// that size before routing, so it stays at the 10 MB default. The routes
|
||||
// that take bigger uploads (WOPI saves, WebDAV, /api/plugin-proxy) are left
|
||||
// out of the proxy's matcher instead.
|
||||
// Response headers are the host's job in a static export (see out/_headers).
|
||||
...(isLite
|
||||
? {}
|
||||
@@ -91,6 +87,15 @@ const nextConfig: NextConfig = {
|
||||
{ key: "Cache-Control", value: "public, max-age=31536000, immutable" },
|
||||
],
|
||||
},
|
||||
// The upload routes proxy.ts's matcher leaves out, with the
|
||||
// headers it sets on every other API response.
|
||||
...["/api/webdav", "/api/plugin-proxy", "/api/wopi/files/:fileId/contents"].map((source) => ({
|
||||
source,
|
||||
headers: [
|
||||
{ key: "X-Content-Type-Options", value: "nosniff" },
|
||||
{ key: "X-Frame-Options", value: "DENY" },
|
||||
],
|
||||
})),
|
||||
];
|
||||
},
|
||||
}),
|
||||
|
||||
@@ -49,12 +49,27 @@ function withMatchedChineseAcceptLanguage(request: NextRequest): NextRequest {
|
||||
return new NextRequest(request, { headers, nextConfig: basePath ? { basePath } : undefined });
|
||||
}
|
||||
|
||||
// Next 16's Proxy always runs on Node.js runtime and route-segment config
|
||||
// (e.g. `export const config = { matcher }`) is no longer allowed in the
|
||||
// proxy file. We replicate the previous matcher inline by short-circuiting
|
||||
// requests for API routes, Next internals and static assets.
|
||||
// Next 16's Proxy always runs on Node.js runtime (a `runtime` in the config
|
||||
// below is refused). The matcher only leaves out the upload routes; API
|
||||
// routes, Next internals and static assets still pass through here and are
|
||||
// short-circuited inline after the setup and CSRF gates.
|
||||
const PROXY_SKIP_PATTERN = /^\/(?:api|_next)(?:\/|$)/;
|
||||
|
||||
/**
|
||||
* Routes the proxy does not run for: the ones that take uploads over 10 MB.
|
||||
* For every request it runs for, Next first copies the body for it, up to
|
||||
* proxyClientMaxBodySize (10 MB) and silently cutting off the rest; raising
|
||||
* that limit made any unauthenticated POST hold up to it in memory. These
|
||||
* handlers gate cross-origin requests themselves (the WOPI file routes are
|
||||
* token-authed) and cap what they read. Next tests the raw and the decoded
|
||||
* path; the proxy is skipped only when both are one of these exactly.
|
||||
* next.config.ts gives them the headers the proxy would have set.
|
||||
* A literal: Next reads it at build time.
|
||||
*/
|
||||
export const config = {
|
||||
matcher: ["/((?!api/webdav$|api/plugin-proxy$|api/wopi/files/[^/]+/contents$).*)"],
|
||||
};
|
||||
|
||||
// What Next serves from public/: a single dotted segment at the root
|
||||
// (/sw.js, /favicon.ico, /manifest.webmanifest) and the branding/,
|
||||
// notification/, demo/ and plugin-sandbox-assets/ asset folders. The upstream matcher treated EVERY path whose
|
||||
|
||||
Reference in New Issue
Block a user