diff --git a/pkg/modules/chromium/chromium.go b/pkg/modules/chromium/chromium.go index 1573613b..c392c1bc 100644 --- a/pkg/modules/chromium/chromium.go +++ b/pkg/modules/chromium/chromium.go @@ -189,10 +189,13 @@ type Options struct { // PDFs with transparency. OmitBackground bool - // AllowedFilePrefixes restricts file:// sub-resource access to only these - // directory prefixes. Applied in listenForEventRequestPaused in addition - // to the global allow/deny lists. Set internally by route handlers, not - // via form data. + // AllowedFilePrefixes restricts file:// sub-resource access to only + // these directory prefixes. Applied in listenForEventRequestPaused in + // addition to the global allow/deny lists. An empty slice + // default-denies every file:// sub-resource, so routes that legitimately + // render local files (HTML, Markdown) must populate this with the + // request working directory while routes that navigate remote URLs + // leave it empty. Set internally by route handlers, not via form data. AllowedFilePrefixes []string } diff --git a/pkg/modules/chromium/events.go b/pkg/modules/chromium/events.go index 4854e9d1..b09ee1b6 100644 --- a/pkg/modules/chromium/events.go +++ b/pkg/modules/chromium/events.go @@ -59,23 +59,18 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio allow = false } - // Additional restriction: if the sub-resource is a file:// URL - // and we have allowed file prefixes, restrict access to only - // those directories. This prevents cross-request file access - // in /tmp. - if allow && strings.HasPrefix(e.Request.URL, "file://") && len(options.allowedFilePrefixes) > 0 { - prefixMatch := false - for _, prefix := range options.allowedFilePrefixes { - if strings.HasPrefix(e.Request.URL, "file://"+prefix) { - prefixMatch = true - break - } - } - - if !prefixMatch { - logger.WarnContext(ctx, fmt.Sprintf("'%s' is not within any allowed file prefix", e.Request.URL)) - allow = false - } + // Sub-resource file:// URLs are opt-in per route. A route + // that renders local files (HTML, Markdown) populates + // allowedFilePrefixes with the request working directory + // so its own assets load while sibling requests' /tmp + // paths stay out of reach. Every other route leaves the + // slice empty; treat that as default-deny so a file:// + // sub-resource that slips past the deny-list (which + // exempts /tmp/) still cannot read the working + // directories of other in-flight conversions. + if allow && strings.HasPrefix(e.Request.URL, "file://") && !isAllowedFileSubResource(e.Request.URL, options.allowedFilePrefixes) { + logger.WarnContext(ctx, fmt.Sprintf("'%s' is not within any allowed file prefix", e.Request.URL)) + allow = false } cctx := chromedp.FromContext(ctx) @@ -250,6 +245,23 @@ func listenForEventResponseReceived( }) } +// isAllowedFileSubResource reports whether a file:// sub-resource URL is +// within at least one prefix. An empty prefix list rejects every +// file:// URL so routes that never populate the list (for example +// /forms/chromium/convert/url) default-deny reads from /tmp/, blocking +// cross-request enumeration. +func isAllowedFileSubResource(rawURL string, allowedFilePrefixes []string) bool { + if len(allowedFilePrefixes) == 0 { + return false + } + for _, prefix := range allowedFilePrefixes { + if strings.HasPrefix(rawURL, "file://"+prefix) { + return true + } + } + return false +} + func shouldCheckResourceHttpStatusCode(rawURL string, ignoreDomains []string) bool { host := hostnameFromURL(rawURL) diff --git a/pkg/modules/chromium/events_test.go b/pkg/modules/chromium/events_test.go index d01bcbe7..63cecbea 100644 --- a/pkg/modules/chromium/events_test.go +++ b/pkg/modules/chromium/events_test.go @@ -61,3 +61,49 @@ func TestShouldCheckResourceHttpStatusCode_NonHTTPURL(t *testing.T) { t.Fatalf("expected data: URL to be checked (no host filtering possible)") } } + +func TestIsAllowedFileSubResource(t *testing.T) { + for _, tc := range []struct { + name string + rawURL string + prefixes []string + want bool + }{ + { + name: "empty prefix list default denies", + rawURL: "file:///tmp/work-uuid/request-uuid/index.html", + prefixes: nil, + want: false, + }, + { + name: "match within the sole prefix", + rawURL: "file:///tmp/work-uuid/request-uuid/index.html", + prefixes: []string{"/tmp/work-uuid/request-uuid"}, + want: true, + }, + { + name: "sibling request directory rejected", + rawURL: "file:///tmp/work-uuid/other-request-uuid/secret.html", + prefixes: []string{"/tmp/work-uuid/request-uuid"}, + want: false, + }, + { + name: "parent tmp directory rejected", + rawURL: "file:///tmp/", + prefixes: []string{"/tmp/work-uuid/request-uuid"}, + want: false, + }, + { + name: "match among several prefixes", + rawURL: "file:///tmp/work-uuid/request-b/asset.css", + prefixes: []string{"/tmp/work-uuid/request-a", "/tmp/work-uuid/request-b"}, + want: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if got := isAllowedFileSubResource(tc.rawURL, tc.prefixes); got != tc.want { + t.Fatalf("isAllowedFileSubResource(%q, %v) = %v, want %v", tc.rawURL, tc.prefixes, got, tc.want) + } + }) + } +}