fix(chromium): default-deny file:// sub-resources when no prefix is allowed

This commit is contained in:
Julien Neuhart
2026-04-21 20:25:34 +02:00
parent 4b192b1498
commit a2a8c42457
3 changed files with 82 additions and 21 deletions

View File

@@ -189,10 +189,13 @@ type Options struct {
// PDFs with transparency. // PDFs with transparency.
OmitBackground bool OmitBackground bool
// AllowedFilePrefixes restricts file:// sub-resource access to only these // AllowedFilePrefixes restricts file:// sub-resource access to only
// directory prefixes. Applied in listenForEventRequestPaused in addition // these directory prefixes. Applied in listenForEventRequestPaused in
// to the global allow/deny lists. Set internally by route handlers, not // addition to the global allow/deny lists. An empty slice
// via form data. // 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 AllowedFilePrefixes []string
} }

View File

@@ -59,23 +59,18 @@ func listenForEventRequestPaused(ctx context.Context, logger *slog.Logger, optio
allow = false allow = false
} }
// Additional restriction: if the sub-resource is a file:// URL // Sub-resource file:// URLs are opt-in per route. A route
// and we have allowed file prefixes, restrict access to only // that renders local files (HTML, Markdown) populates
// those directories. This prevents cross-request file access // allowedFilePrefixes with the request working directory
// in /tmp. // so its own assets load while sibling requests' /tmp
if allow && strings.HasPrefix(e.Request.URL, "file://") && len(options.allowedFilePrefixes) > 0 { // paths stay out of reach. Every other route leaves the
prefixMatch := false // slice empty; treat that as default-deny so a file://
for _, prefix := range options.allowedFilePrefixes { // sub-resource that slips past the deny-list (which
if strings.HasPrefix(e.Request.URL, "file://"+prefix) { // exempts /tmp/) still cannot read the working
prefixMatch = true // directories of other in-flight conversions.
break 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
if !prefixMatch {
logger.WarnContext(ctx, fmt.Sprintf("'%s' is not within any allowed file prefix", e.Request.URL))
allow = false
}
} }
cctx := chromedp.FromContext(ctx) 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 { func shouldCheckResourceHttpStatusCode(rawURL string, ignoreDomains []string) bool {
host := hostnameFromURL(rawURL) host := hostnameFromURL(rawURL)

View File

@@ -61,3 +61,49 @@ func TestShouldCheckResourceHttpStatusCode_NonHTTPURL(t *testing.T) {
t.Fatalf("expected data: URL to be checked (no host filtering possible)") 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)
}
})
}
}