diff --git a/pkg/modules/exiftool/exiftool.go b/pkg/modules/exiftool/exiftool.go index 33b22ad5..cde6f6a5 100644 --- a/pkg/modules/exiftool/exiftool.go +++ b/pkg/modules/exiftool/exiftool.go @@ -268,15 +268,18 @@ func (engine *ExifTool) WriteMetadata(ctx context.Context, logger *slog.Logger, } // Filter user-supplied metadata to prevent ExifTool pseudo-tags from - // triggering dangerous side effects like file renames, moves, or link - // creation. Comparison is case-insensitive because ExifTool processes - // tag names case-insensitively. + // triggering dangerous side effects like file renames, moves, link + // creation, or permission changes. Comparison is case-insensitive + // because ExifTool processes tag names case-insensitively, and group + // prefixes are stripped because ExifTool treats "System:FileName" the + // same as "FileName". // See https://exiftool.org/TagNames/Extra.html. dangerousTags := []string{ - "FileName", // Writing this triggers a file rename in ExifTool - "Directory", // Writing this triggers a file move in ExifTool - "HardLink", // Writing this creates a hard link in ExifTool - "SymLink", // Writing this creates a symbolic link in ExifTool + "FileName", // Writing this triggers a file rename in ExifTool + "Directory", // Writing this triggers a file move in ExifTool + "HardLink", // Writing this creates a hard link in ExifTool + "SymLink", // Writing this creates a symbolic link in ExifTool + "FilePermissions", // Writing this changes the file's permissions } // Reject metadata keys containing characters that could inject ExifTool // stdin arguments. ExifTool uses a line-based stdin protocol; a newline @@ -293,8 +296,16 @@ func (engine *ExifTool) WriteMetadata(ctx context.Context, logger *slog.Logger, } for key := range metadata { + // Strip ExifTool group prefixes (e.g., "System:FileName" → + // "FileName") before comparing. ExifTool allows leading group + // names separated by colons, and treats the prefixed and bare + // forms identically. + bare := key + if i := strings.LastIndex(key, ":"); i >= 0 { + bare = key[i+1:] + } for _, tag := range dangerousTags { - if strings.EqualFold(key, tag) { + if strings.EqualFold(bare, tag) { delete(metadata, key) } } diff --git a/test/integration/features/pdfengines_metadata.feature b/test/integration/features/pdfengines_metadata.feature index c260ba94..00d780b6 100644 --- a/test/integration/features/pdfengines_metadata.feature +++ b/test/integration/features/pdfengines_metadata.feature @@ -123,6 +123,26 @@ Feature: /forms/pdfengines/metadata/{write|read} At least one PDF engine cannot process the requested metadata """ + Scenario: POST /forms/pdfengines/metadata/write (Reject Group-Prefixed Dangerous Tag) + # Regression: ExifTool treats "System:FileName" identically to "FileName". + # The dangerous-tag blocklist must strip group prefixes before comparing, + # otherwise the attacker renames/moves files with a single HTTP request. + Given I have a default Gotenberg container + When I make a "POST" request to Gotenberg at the "/forms/pdfengines/metadata/write" endpoint with the following form data and header(s): + | files | testdata/page_1.pdf | file | + | metadata | {"System:FileName":"stolen.pdf","System:Directory":"/tmp","Author":"legit"} | field | + | Gotenberg-Output-Filename | foo | header | + Then the response status code should be 200 + Then the response header "Content-Type" should be "application/pdf" + Then there should be 1 PDF(s) in the response + When I make a "POST" request to Gotenberg at the "/forms/pdfengines/metadata/read" endpoint with the following form data and header(s): + | files | teststore/foo.pdf | file | + Then the response status code should be 200 + Then the response body should contain string: + """ + "Author":"legit" + """ + Scenario: POST /forms/pdfengines/metadata/read (Bad Request) Given I have a default Gotenberg container When I make a "POST" request to Gotenberg at the "/forms/pdfengines/metadata/read" endpoint with the following form data and header(s):