diff --git a/pkg/modules/api/middlewares.go b/pkg/modules/api/middlewares.go index 7b1d7def..73b295c6 100644 --- a/pkg/modules/api/middlewares.go +++ b/pkg/modules/api/middlewares.go @@ -7,7 +7,6 @@ import ( "fmt" "log/slog" "net/http" - "path/filepath" "strings" "time" @@ -150,9 +149,16 @@ func outputFilenameMiddleware() echo.MiddlewareFunc { return func(next echo.HandlerFunc) echo.HandlerFunc { return func(c echo.Context) error { filename := c.Request().Header.Get("Gotenberg-Output-Filename") + // Keep only the last path segment, so that a caller cannot name an + // output file after a path. // See https://github.com/gotenberg/gotenberg/issues/1227. + // + // [filepath.Base] alone is not enough: on Linux it does not treat a + // backslash as a separator, and this value reaches archive entry + // names. Use the same sanitizer as the other caller-supplied + // filenames. if filename != "" { - filename = filepath.Base(filename) + filename = sanitizeFilename(filename) } c.Set("outputFilename", filename) // Call the next middleware in the chain. diff --git a/pkg/modules/api/middlewares_test.go b/pkg/modules/api/middlewares_test.go index ff129c0c..b189c44e 100644 --- a/pkg/modules/api/middlewares_test.go +++ b/pkg/modules/api/middlewares_test.go @@ -10,6 +10,53 @@ import ( "github.com/labstack/echo/v4" ) +// TestOutputFilenameMiddleware pins the sanitizing of the +// "Gotenberg-Output-Filename" header. The value reaches archive entry names and +// a Content-Disposition header, so a path separator must never survive it. +// See https://github.com/gotenberg/gotenberg/issues/1227 and +// GHSA-hwc4-gmrw-5222. +func TestOutputFilenameMiddleware(t *testing.T) { + for _, tc := range []struct { + name string + header string + want string + }{ + {"no header", "", ""}, + {"plain filename", "foo", "foo"}, + {"POSIX path", "/tmp/foo", "foo"}, + {"POSIX traversal", "../../../etc/passwd", "passwd"}, + {"Windows traversal", `..\..\..\..\Windows\System32\evil`, "evil"}, + {"rooted Windows path", `C:\Windows\Temp\evil`, "evil"}, + {"mixed separators", `a/b\c`, "c"}, + {"trailing separator", "/tmp/", ""}, + {"bare dot dot", "..", ".."}, + {"control characters", "fo\x01o\x7f", "foo"}, + } { + t.Run(tc.name, func(t *testing.T) { + handler := outputFilenameMiddleware()(func(c echo.Context) error { return nil }) + + req := httptest.NewRequest(http.MethodPost, "/", nil) + if tc.header != "" { + req.Header.Set("Gotenberg-Output-Filename", tc.header) + } + c := echo.New().NewContext(req, httptest.NewRecorder()) + + err := handler(c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + got, ok := c.Get("outputFilename").(string) + if !ok { + t.Fatal("outputFilename is not set as a string") + } + if got != tc.want { + t.Errorf("outputFilename = %q, want %q", got, tc.want) + } + }) + } +} + func TestHardTimeoutMiddleware_MissingLoggerReturnsErrorInsteadOfPanicking(t *testing.T) { mw := hardTimeoutMiddleware(100 * time.Millisecond) handler := mw(func(c echo.Context) error { return nil }) diff --git a/test/integration/features/chromium_convert_html.feature b/test/integration/features/chromium_convert_html.feature index 786cc93f..77f60051 100644 --- a/test/integration/features/chromium_convert_html.feature +++ b/test/integration/features/chromium_convert_html.feature @@ -813,8 +813,40 @@ Feature: /forms/chromium/convert/html """ # See https://github.com/gotenberg/gotenberg/issues/1130. + # A backslash is not a path separator on Linux, so filepath.Base leaves it in + # place and it reaches the archive entry names. See GHSA-hwc4-gmrw-5222. @split @output-filename + Scenario: POST /forms/chromium/convert/html (Split Windows Path As Output Filename) + Given I have a default Gotenberg container + When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): + | files | testdata/pages-3-html/index.html | file | + | splitMode | intervals | field | + | splitSpan | 2 | field | + | Gotenberg-Output-Filename | ..\\..\\..\\Windows\\System32\\foo | header | + Then the response status code should be 200 + Then the response header "Content-Type" should be "application/zip" + Then there should be 2 PDF(s) in the response + Then there should be the following file(s) in the response: + | foo.zip | + | foo_0.pdf | + | foo_1.pdf | + + Scenario: POST /forms/chromium/convert/html (Split Rooted Windows Path As Output Filename) + Given I have a default Gotenberg container + When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): + | files | testdata/pages-3-html/index.html | file | + | splitMode | intervals | field | + | splitSpan | 2 | field | + | Gotenberg-Output-Filename | C:\\Windows\\Temp\\foo | header | + Then the response status code should be 200 + Then the response header "Content-Type" should be "application/zip" + Then there should be 2 PDF(s) in the response + Then there should be the following file(s) in the response: + | foo.zip | + | foo_0.pdf | + | foo_1.pdf | + Scenario: POST /forms/chromium/convert/html (Split Output Filename) Given I have a default Gotenberg container When I make a "POST" request to Gotenberg at the "/forms/chromium/convert/html" endpoint with the following form data and header(s): diff --git a/test/integration/features/output_filename.feature b/test/integration/features/output_filename.feature index 63126786..ab14e832 100644 --- a/test/integration/features/output_filename.feature +++ b/test/integration/features/output_filename.feature @@ -22,6 +22,17 @@ Feature: Output Filename Then there should be the following file(s) in the response: | foo.zip | + # See GHSA-hwc4-gmrw-5222. + Scenario: Windows Path As Filename + Given I have a default Gotenberg container + When I make a "POST" request to Gotenberg at the "/forms/pdfengines/flatten" endpoint with the following form data and header(s): + | files | testdata/page_1.pdf | file | + | Gotenberg-Output-Filename | C:\\Windows\\Temp\\foo | header | + Then the response status code should be 200 + Then the response header "Content-Type" should be "application/pdf" + Then there should be the following file(s) in the response: + | foo.pdf | + # See https://github.com/gotenberg/gotenberg/issues/1227. Scenario: Path As Filename Given I have a default Gotenberg container