diff --git a/server/go.mod b/server/go.mod index 8a36326d30..f98b5f292e 100644 --- a/server/go.mod +++ b/server/go.mod @@ -74,6 +74,7 @@ require ( golang.org/x/net v0.43.0 golang.org/x/sync v0.17.0 golang.org/x/term v0.34.0 + golang.org/x/text v0.29.0 gopkg.in/mail.v2 v2.3.1 gopkg.in/yaml.v3 v3.0.1 ) @@ -223,7 +224,6 @@ require ( golang.org/x/exp v0.0.0-20250506013437-ce4c2cf36ca6 // indirect golang.org/x/mod v0.27.0 // indirect golang.org/x/sys v0.35.0 // indirect - golang.org/x/text v0.29.0 // indirect golang.org/x/tools v0.36.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250512202823-5a2f75b736a9 // indirect google.golang.org/grpc v1.72.0 // indirect diff --git a/server/platform/services/imageproxy/local.go b/server/platform/services/imageproxy/local.go index 33b8169e2e..af8804d874 100644 --- a/server/platform/services/imageproxy/local.go +++ b/server/platform/services/imageproxy/local.go @@ -19,12 +19,15 @@ import ( "strings" "time" + "golang.org/x/text/encoding/unicode" + "golang.org/x/text/transform" + "github.com/mattermost/mattermost/server/public/shared/mlog" ) var imageContentTypes = []string{ "image/bmp", "image/cgm", "image/g3fax", "image/gif", "image/ief", "image/jp2", - "image/jpeg", "image/jpg", "image/pict", "image/png", "image/prs.btif", "image/svg+xml", + "image/jpeg", "image/jpg", "image/pict", "image/png", "image/prs.btif", "image/tiff", "image/vnd.adobe.photoshop", "image/vnd.djvu", "image/vnd.dwg", "image/vnd.dxf", "image/vnd.fastbidsheet", "image/vnd.fpx", "image/vnd.fst", "image/vnd.fujixerox.edmics-mmr", "image/vnd.fujixerox.edmics-rlc", @@ -162,22 +165,30 @@ func (backend *LocalBackend) ServeImage(w http.ResponseWriter, req *http.Request copyHeader(w.Header(), resp.Header, "Cache-Control", "Last-Modified", "Expires", "Etag", "Link") - if should304(req, resp) { - w.WriteHeader(http.StatusNotModified) + // Wrap the body in a bufio.Reader so we can peek at bytes for + // content-type detection without consuming the stream. + b := bufio.NewReaderSize(resp.Body, contentPeekSize) + resp.Body = io.NopCloser(b) + + if isSVGContent(b) { + http.Error(w, msgNotAllowed, http.StatusForbidden) return } contentType, _, _ := mime.ParseMediaType(resp.Header.Get("Content-Type")) if contentType == "" || contentType == "application/octet-stream" || contentType == "binary/octet-stream" { - // try to detect content type - b := bufio.NewReader(resp.Body) - resp.Body = io.NopCloser(b) contentType = peekContentType(b) } if resp.ContentLength != 0 && !contentTypeMatches(imageContentTypes, contentType) { http.Error(w, msgNotAllowed, http.StatusForbidden) return } + + if should304(req, resp) { + w.WriteHeader(http.StatusNotModified) + return + } + w.Header().Set("Content-Type", contentType) copyHeader(w.Header(), resp.Header, "Content-Length") @@ -248,6 +259,41 @@ func peekContentType(p *bufio.Reader) string { return http.DetectContentType(byt) } +// contentPeekSize is the number of bytes read ahead for content inspection. +// It must match the bufio.Reader buffer size created in ServeImage. +const contentPeekSize = 8192 + +// isSVGContent peeks at the first contentPeekSize bytes of p and reports whether +// they contain SVG markers. UTF-16 encoded content (identified by a BOM) is +// decoded to ASCII before scanning. +func isSVGContent(p *bufio.Reader) bool { + byt, err := p.Peek(contentPeekSize) + if err != nil && err != bufio.ErrBufferFull && err != io.EOF { + return false + } + if len(byt) == 0 { + return false + } + + // UseBOM selects endianness from a BOM when present (0xFF 0xFE → LE, + // 0xFE 0xFF → BE), defaulting to LE otherwise. + enc := unicode.UTF16(unicode.LittleEndian, unicode.UseBOM) + if decoded, _, decodeErr := transform.Bytes(enc.NewDecoder(), byt); decodeErr == nil { + lower := strings.ToLower(string(decoded)) + if strings.Contains(lower, "`) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + + w.WriteHeader(http.StatusOK) + w.Write(body) + }) + + mock := httptest.NewServer(handler) + defer mock.Close() + + proxy := makeTestLocalProxy() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodGet, "", nil) require.NoError(t, err) + proxy.GetImage(recorder, request, mock.URL+"/image.png") + resp := recorder.Result() + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) + + t.Run("XML-based SVG with image/png content type", func(t *testing.T) { + body := []byte(``) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + + w.WriteHeader(http.StatusOK) + w.Write(body) + }) + + mock := httptest.NewServer(handler) + defer mock.Close() + + proxy := makeTestLocalProxy() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodGet, "", nil) + require.NoError(t, err) + proxy.GetImage(recorder, request, mock.URL+"/image.png") + resp := recorder.Result() + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) + + t.Run("UTF-16 LE BOM SVG with image/png content type", func(t *testing.T) { + // Build a UTF-16 LE payload with BOM: 0xFF 0xFE followed by each ASCII + // character of the SVG tag as a two-byte little-endian code unit. + svgASCII := `` + body := []byte{0xFF, 0xFE} // UTF-16 LE BOM + for _, c := range svgASCII { + body = append(body, byte(c), 0x00) + } + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + w.WriteHeader(http.StatusOK) + w.Write(body) + }) + + mock := httptest.NewServer(handler) + defer mock.Close() + + proxy := makeTestLocalProxy() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodGet, "", nil) + require.NoError(t, err) + proxy.GetImage(recorder, request, mock.URL+"/image.png") + resp := recorder.Result() + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) + + t.Run("UTF-16 BE BOM SVG with image/png content type", func(t *testing.T) { + // Build a UTF-16 BE payload with BOM: 0xFE 0xFF followed by each ASCII + // character as a two-byte big-endian code unit. + svgASCII := `` + body := []byte{0xFE, 0xFF} // UTF-16 BE BOM + for _, c := range svgASCII { + body = append(body, 0x00, byte(c)) + } + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + w.WriteHeader(http.StatusOK) + w.Write(body) + }) + + mock := httptest.NewServer(handler) + defer mock.Close() + + proxy := makeTestLocalProxy() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodGet, "", nil) + require.NoError(t, err) + proxy.GetImage(recorder, request, mock.URL+"/image.png") + resp := recorder.Result() + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + }) + + t.Run("SVG body with leading whitespace prefix", func(t *testing.T) { + prefix := bytes.Repeat([]byte(" "), 600) + body := append(prefix, []byte(``)...) + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/png") + + w.WriteHeader(http.StatusOK) + w.Write(body) + }) + + mock := httptest.NewServer(handler) + defer mock.Close() + + proxy := makeTestLocalProxy() + + recorder := httptest.NewRecorder() + request, err := http.NewRequest(http.MethodGet, "", nil) + require.NoError(t, err) + proxy.GetImage(recorder, request, mock.URL+"/image.png") + resp := recorder.Result() + + assert.Equal(t, http.StatusForbidden, resp.StatusCode) }) t.Run("Redirect", func(t *testing.T) {