diff --git a/pkg/utils/image.go b/pkg/utils/image.go index b61643fac6..0ef13e9303 100644 --- a/pkg/utils/image.go +++ b/pkg/utils/image.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "regexp" + "strings" "time" ) @@ -32,7 +33,32 @@ func ProcessImageInput(ctx context.Context, imageInput string) ([]byte, error) { } // assume input is a URL. Read it. - return ReadImageFromURL(ctx, imageInput) + d, err := ReadImageFromURL(ctx, imageInput) + if err != nil { + return nil, err + } + + if err := validateImageData(d); err != nil { + return nil, err + } + + return d, nil +} + +// validateImageData rejects HTML content, which is not a valid image and would +// execute as a document if served back to a browser. SVG (detected as XML or +// plain text) is still accepted and sandboxed on output by ServeImage. +func validateImageData(data []byte) error { + if len(data) == 0 { + return nil + } + + contentType := http.DetectContentType(data) + if strings.HasPrefix(contentType, "text/html") { + return fmt.Errorf("unsupported image content type %q", contentType) + } + + return nil } // ReadImageFromURL returns image data from a URL @@ -97,6 +123,10 @@ func ProcessBase64Image(imageString string) ([]byte, error) { return nil, err } + if err := validateImageData(imageData); err != nil { + return nil, err + } + return imageData, nil } @@ -112,10 +142,22 @@ func GetBase64StringFromData(data []byte) string { func ServeImage(w http.ResponseWriter, r *http.Request, image []byte) { contentType := http.DetectContentType(image) + + // SVG images are detected as XML or plain text; serve them as SVG so they + // render. The sandboxing CSP below prevents any embedded script running. if contentType == "text/xml; charset=utf-8" || contentType == "text/plain; charset=utf-8" { contentType = "image/svg+xml" + } else if strings.HasPrefix(contentType, "text/") { + // any other text type (e.g. HTML) is not a valid image - never render it + contentType = "application/octet-stream" + w.Header().Set("Content-Disposition", "attachment") } + // sandbox every image response so a stored SVG cannot execute script or + // exfiltrate data; harmless for raster images. + w.Header().Set("Content-Security-Policy", "default-src 'none'; img-src data:; style-src 'unsafe-inline'; sandbox") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("Content-Type", contentType) ServeStaticContent(w, r, image) } diff --git a/pkg/utils/image_test.go b/pkg/utils/image_test.go new file mode 100644 index 0000000000..dba640697b --- /dev/null +++ b/pkg/utils/image_test.go @@ -0,0 +1,97 @@ +package utils + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// a minimal valid 1x1 PNG +var pngImage = []byte{ + 0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d, + 0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, 0x89, 0x00, 0x00, 0x00, + 0x0a, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00, + 0x05, 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x49, + 0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82, +} + +func TestServeImage(t *testing.T) { + const wantCSP = "default-src 'none'; img-src data:; style-src 'unsafe-inline'; sandbox" + + tests := []struct { + name string + image []byte + wantContentType string + wantAttachment bool + }{ + { + name: "png is served as-is", + image: pngImage, + wantContentType: "image/png", + }, + { + name: "svg is served as image/svg+xml so it renders", + image: []byte(``), + wantContentType: "image/svg+xml", + }, + { + name: "svg with xml prolog is served as image/svg+xml", + image: []byte(``), + wantContentType: "image/svg+xml", + }, + { + name: "html is never rendered - forced to an attachment", + image: []byte(``), + wantContentType: "application/octet-stream", + wantAttachment: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/image", nil) + + ServeImage(w, r, tt.image) + + if got := w.Header().Get("Content-Type"); got != tt.wantContentType { + t.Errorf("Content-Type = %q, want %q", got, tt.wantContentType) + } + if got := w.Header().Get("Content-Security-Policy"); got != wantCSP { + t.Errorf("Content-Security-Policy = %q, want %q", got, wantCSP) + } + if got := w.Header().Get("X-Content-Type-Options"); got != "nosniff" { + t.Errorf("X-Content-Type-Options = %q, want %q", got, "nosniff") + } + gotAttachment := strings.Contains(w.Header().Get("Content-Disposition"), "attachment") + if gotAttachment != tt.wantAttachment { + t.Errorf("Content-Disposition attachment = %v, want %v (got %q)", gotAttachment, tt.wantAttachment, w.Header().Get("Content-Disposition")) + } + }) + } +} + +func TestValidateImageData(t *testing.T) { + tests := []struct { + name string + data []byte + wantErr bool + }{ + {name: "empty is allowed", data: nil, wantErr: false}, + {name: "png is allowed", data: pngImage, wantErr: false}, + {name: "svg is allowed", data: []byte(``), wantErr: false}, + {name: "html is rejected", data: []byte(``), wantErr: true}, + {name: "html without doctype is rejected", data: []byte(`x`), wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateImageData(tt.data) + if (err != nil) != tt.wantErr { + t.Errorf("validateImageData() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +}