diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-01 15:11:08 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-05-01 15:11:08 +0300 |
commit | c14adba064aa86114dc43cae657212a4b19d6189 (patch) | |
tree | bf1e9a630b154317aaa7e61e6ba1ee6f7349eca7 /workhorse | |
parent | c6cc9bc94e23e01a01ed191aba993ccf2b443680 (diff) |
Add latest changes from gitlab-org/security/gitlab@15-9-stable-ee
Diffstat (limited to 'workhorse')
-rw-r--r-- | workhorse/internal/headers/content_headers.go | 32 | ||||
-rw-r--r-- | workhorse/internal/headers/content_headers_test.go | 56 | ||||
-rw-r--r-- | workhorse/internal/senddata/contentprocessor/contentprocessor_test.go | 4 | ||||
-rw-r--r-- | workhorse/testdata/index.xhtml | 9 | ||||
-rw-r--r-- | workhorse/testdata/test.xml | 6 | ||||
-rw-r--r-- | workhorse/testdata/xml.svg | 7 |
6 files changed, 107 insertions, 7 deletions
diff --git a/workhorse/internal/headers/content_headers.go b/workhorse/internal/headers/content_headers.go index 854cc8abddd..54c7c1bdd95 100644 --- a/workhorse/internal/headers/content_headers.go +++ b/workhorse/internal/headers/content_headers.go @@ -1,6 +1,7 @@ package headers import ( + "mime" "net/http" "regexp" @@ -13,8 +14,9 @@ var ( imageTypeRegex = regexp.MustCompile(`^image/*`) svgMimeTypeRegex = regexp.MustCompile(`^image/svg\+xml$`) - textTypeRegex = regexp.MustCompile(`^text/*`) - + textTypeRegex = regexp.MustCompile(`^text/*`) + xmlTypeRegex = regexp.MustCompile(`^text/xml`) + xhtmlTypeRegex = regexp.MustCompile(`^text/html`) videoTypeRegex = regexp.MustCompile(`^video/*`) pdfTypeRegex = regexp.MustCompile(`application\/pdf`) @@ -26,6 +28,8 @@ var ( // Mime types that can't be inlined. Usually subtypes of main types var forbiddenInlineTypes = []*regexp.Regexp{svgMimeTypeRegex} +var htmlRenderingTypes = []*regexp.Regexp{xmlTypeRegex, xhtmlTypeRegex} + // Mime types that can be inlined. We can add global types like "image/" or // specific types like "text/plain". If there is a specific type inside a global // allowed type that can't be inlined we must add it to the forbiddenInlineTypes var. @@ -38,12 +42,28 @@ const ( textPlainContentType = "text/plain; charset=utf-8" attachmentDispositionText = "attachment" inlineDispositionText = "inline" + dummyFilename = "blob" ) func SafeContentHeaders(data []byte, contentDisposition string) (string, string) { - contentType := safeContentType(data) + detectedContentType := detectContentType(data) + + contentType := safeContentType(detectedContentType) contentDisposition = safeContentDisposition(contentType, contentDisposition) + // Some browsers will render XML inline unless a filename directive is provided with a non-xml file extension + // This overrides the filename directive in the case of XML data + for _, element := range htmlRenderingTypes { + if isType(detectedContentType, element) { + disposition, directives, err := mime.ParseMediaType(contentDisposition) + if err == nil { + directives["filename"] = dummyFilename + contentDisposition = mime.FormatMediaType(disposition, directives) + break + } + } + } + // Set attachments to application/octet-stream since browsers can do // a better job distinguishing certain types (for example: ZIP files // vs. Microsoft .docx files). However, browsers may safely render SVGs even @@ -56,15 +76,17 @@ func SafeContentHeaders(data []byte, contentDisposition string) (string, string) return contentType, contentDisposition } -func safeContentType(data []byte) string { +func detectContentType(data []byte) string { // Special case for svg because DetectContentType detects it as text if svg.Is(data) { return svgContentType } // Override any existing Content-Type header from other ResponseWriters - contentType := http.DetectContentType(data) + return http.DetectContentType(data) +} +func safeContentType(contentType string) string { // http.DetectContentType does not support JavaScript and would only // return text/plain. But for cautionary measures, just in case they start supporting // it down the road and start returning application/javascript, we want to handle it now diff --git a/workhorse/internal/headers/content_headers_test.go b/workhorse/internal/headers/content_headers_test.go new file mode 100644 index 00000000000..7cfce335d88 --- /dev/null +++ b/workhorse/internal/headers/content_headers_test.go @@ -0,0 +1,56 @@ +package headers + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func fileContents(fileName string) []byte { + fileContents, _ := os.ReadFile(fileName) + return fileContents +} + +func TestHeaders(t *testing.T) { + tests := []struct { + desc string + fileContents []byte + expectedContentType string + expectedContentDisposition string + }{ + { + desc: "XML file", + fileContents: fileContents("../../testdata/test.xml"), + expectedContentType: "text/plain; charset=utf-8", + expectedContentDisposition: "inline; filename=blob", + }, + { + desc: "XHTML file", + fileContents: fileContents("../../testdata/index.xhtml"), + expectedContentType: "text/plain; charset=utf-8", + expectedContentDisposition: "inline; filename=blob", + }, + { + desc: "svg+xml file", + fileContents: fileContents("../../testdata/xml.svg"), + expectedContentType: "image/svg+xml", + expectedContentDisposition: "attachment", + }, + { + desc: "text file", + fileContents: []byte(`a text file`), + expectedContentType: "text/plain; charset=utf-8", + expectedContentDisposition: "inline", + }, + } + + for _, test := range tests { + t.Run(test.desc, func(t *testing.T) { + contentType, newContentDisposition := SafeContentHeaders(test.fileContents, "") + + require.Equal(t, test.expectedContentType, contentType) + require.Equal(t, test.expectedContentDisposition, newContentDisposition) + }) + } +} diff --git a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go index b04263de6b9..e863935be6f 100644 --- a/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go +++ b/workhorse/internal/senddata/contentprocessor/contentprocessor_test.go @@ -51,13 +51,13 @@ func TestSetProperContentTypeAndDisposition(t *testing.T) { { desc: "HTML type", contentType: "text/plain; charset=utf-8", - contentDisposition: "inline", + contentDisposition: "inline; filename=blob", body: "<html><body>Hello world!</body></html>", }, { desc: "Javascript within HTML type", contentType: "text/plain; charset=utf-8", - contentDisposition: "inline", + contentDisposition: "inline; filename=blob", body: "<script>alert(\"foo\")</script>", }, { diff --git a/workhorse/testdata/index.xhtml b/workhorse/testdata/index.xhtml new file mode 100644 index 00000000000..1dd50a70e69 --- /dev/null +++ b/workhorse/testdata/index.xhtml @@ -0,0 +1,9 @@ +<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN" +"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd"> +<html xmlns="http://www.w3.org/1999/xhtml"> +<head> + <title>Title of document</title> +</head> +<body> +</body> +</html> diff --git a/workhorse/testdata/test.xml b/workhorse/testdata/test.xml new file mode 100644 index 00000000000..54b94e62355 --- /dev/null +++ b/workhorse/testdata/test.xml @@ -0,0 +1,6 @@ +<?xml version="1.0" encoding="utf-8"?> +<html> + <head></head> + <body> + </body> +</html>
\ No newline at end of file diff --git a/workhorse/testdata/xml.svg b/workhorse/testdata/xml.svg new file mode 100644 index 00000000000..c41c4c44b49 --- /dev/null +++ b/workhorse/testdata/xml.svg @@ -0,0 +1,7 @@ +<?xml version="1.0" standalone="no"?> +<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"> + +<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg"> + <polygon id="triangle" points="0,0 0,50 50,0" fill="#009900" stroke="#004400"/> + <div>hello this is html</div> +</svg> |