fix(security): contain logo, favicon and PDF-logo unlinks to their upload directories

Settings > Branding persisted logo_url / favicon_url verbatim and on clear
unlinked path.join(storage, url) behind a startsWith('/uploads/logos/')
check, which '..' segments pass. The business-profile PDF logo did the same
behind a /pdf-logo-\d+\./ marker test, and used absolute values as given.
Either let a settings.edit or settings.banking holder delete any file the
process can reach.

Both now resolve through helpers in utils/safePath that only ever name a
flat leaf inside the fixed directory. The /favicon.ico streamer is narrowed
the same way: it contained to the whole uploads/ root, which also holds
signed contracts and transfer files.
This commit is contained in:
Paul Nothaft
2026-09-03 10:44:55 +02:00
parent 0ca0e4a922
commit 3e46530072
5 changed files with 124 additions and 22 deletions
+7 -2
View File
@@ -791,10 +791,15 @@ app.get(
// whereas Firefox/Chrome do — so a 302 worked everywhere except
// Safari. sendFile sets the right content-type from the extension.
const rel = String(url).replace(/^\/+/, '').replace(/^uploads\//, '');
// Containment is the two public asset trees, not the whole uploads/
// root: that root also holds signed contracts and client transfer
// files, and the favicon URL is an admin-writable setting, so the
// wider check let `/uploads/contracts/signed/<file>` be served here
// unauthenticated with a day of cache.
const uploadsRoot = path.resolve(path.join(storagePath, 'uploads'));
const resolved = path.resolve(path.join(uploadsRoot, rel));
// Path containment — never serve outside the uploads dir.
if (resolved.startsWith(uploadsRoot + path.sep) && fs.existsSync(resolved)) {
const servableRoots = ['favicons', 'logos'].map((d) => path.join(uploadsRoot, d) + path.sep);
if (servableRoots.some((root) => resolved.startsWith(root)) && fs.existsSync(resolved)) {
// This route streams the file directly, bypassing the secureStatic
// middleware — so re-apply its SVG hardening here. An admin-uploaded
// SVG favicon could contain <script>; served at the top-level