From 93d0103585372433e18b351bb16edf4c383932d3 Mon Sep 17 00:00:00 2001 From: Julien Neuhart Date: Tue, 5 May 2026 21:13:35 +0200 Subject: [PATCH] fix(api): strip backslash separators from supplied filenames --- pkg/modules/api/context.go | 37 +++++++++++++++++---- pkg/modules/api/context_test.go | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 7 deletions(-) diff --git a/pkg/modules/api/context.go b/pkg/modules/api/context.go index a0bc3d2f..c91b3551 100644 --- a/pkg/modules/api/context.go +++ b/pkg/modules/api/context.go @@ -348,10 +348,13 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys ) } - // Avoid directory traversal and make sure filename characters are - // normalized. + // Strip path separators (including backslashes) and control + // characters, then NFC-normalize. Defends against directory + // traversal in the on-disk name and Windows-side Zip Slip + // when the original filename is later embedded in an output + // zip entry. // See: https://github.com/gotenberg/gotenberg/issues/662. - filename = norm.NFC.String(filepath.Base(filename)) + filename = sanitizeFilename(filename) // Use a UUID-based name on disk to avoid filesystem // NAME_MAX limits with long filenames. @@ -428,10 +431,12 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys // This will ensure we do not exceed the body limit. reader := &trackingReader{R: in, AddReadBytes: addReadBytes} - // Avoid directory traversal and make sure filename characters are - // normalized. + // Strip path separators (including backslashes) and control + // characters, then NFC-normalize. Defends against directory + // traversal in the on-disk name and Windows-side Zip Slip when the + // original filename is later embedded in an output zip entry. // See: https://github.com/gotenberg/gotenberg/issues/662. - filename := norm.NFC.String(filepath.Base(fh.Filename)) + filename := sanitizeFilename(fh.Filename) // Use a UUID-based name on disk to avoid filesystem // NAME_MAX limits with long filenames. @@ -469,7 +474,7 @@ func newContext(echoCtx echo.Context, logger *slog.Logger, fs *gotenberg.FileSys return ctx, cancel, fmt.Errorf("copy to disk: %w", err) } // Track files by field name - filename := norm.NFC.String(filepath.Base(fh.Filename)) + filename := sanitizeFilename(fh.Filename) filePath := ctx.files[filename] ctx.filesByField[fieldName] = append(ctx.filesByField[fieldName], filePath) } @@ -658,3 +663,21 @@ func (ctx *Context) OutputFilename(outputPath string) string { return fmt.Sprintf("%s%s", filename, filepath.Ext(outputPath)) } + +// sanitizeFilename strips path separators (including backslashes, which +// [filepath.Base] ignores on Linux) and control characters from a +// caller-supplied filename, then NFC-normalizes the result. This prevents a +// Windows-side Zip Slip when an output zip is extracted by a permissive +// extractor that interprets '\' as a path separator. +func sanitizeFilename(name string) string { + if i := strings.LastIndexAny(name, `/\`); i >= 0 { + name = name[i+1:] + } + name = strings.Map(func(r rune) rune { + if r < 0x20 || r == 0x7f { + return -1 + } + return r + }, name) + return norm.NFC.String(name) +} diff --git a/pkg/modules/api/context_test.go b/pkg/modules/api/context_test.go index 9ea502a4..d9f1543c 100644 --- a/pkg/modules/api/context_test.go +++ b/pkg/modules/api/context_test.go @@ -69,3 +69,60 @@ func TestNewContext_Cancellation(t *testing.T) { t.Fatal("expected context to be cancelled after request context cancellation, but it timed out") } } + +func TestSanitizeFilename(t *testing.T) { + for _, tc := range []struct { + scenario string + input string + expect string + }{ + { + scenario: "plain filename is unchanged", + input: "report.pdf", + expect: "report.pdf", + }, + { + scenario: "POSIX traversal is stripped", + input: "../../etc/passwd", + expect: "passwd", + }, + { + scenario: "Windows traversal with backslashes is stripped", + input: `..\..\..\..\Windows\System32\evil.pdf`, + expect: "evil.pdf", + }, + { + scenario: "mixed separators take the last segment", + input: `foo/bar\baz.pdf`, + expect: "baz.pdf", + }, + { + scenario: "control characters are dropped", + input: "evil\x00\x07\x1f\x7f.pdf", + expect: "evil.pdf", + }, + { + scenario: "NFC normalization collapses decomposed sequences", + // "e" + combining acute accent -> precomposed "é". + input: "café.pdf", + expect: "café.pdf", + }, + { + scenario: "trailing backslash yields empty name", + input: `foo\`, + expect: "", + }, + { + scenario: "empty input yields empty name", + input: "", + expect: "", + }, + } { + t.Run(tc.scenario, func(t *testing.T) { + got := sanitizeFilename(tc.input) + if got != tc.expect { + t.Errorf("sanitizeFilename(%q) = %q, want %q", tc.input, got, tc.expect) + } + }) + } +}