Storage: Mime type detection (#52512)

* Storage: implement mime type detection

* lint

pkg/services/store/go-is-svg/svg.go

@@ -0,0 +1,62 @@
+// The MIT License
+//
+//Copyright (c) 2016 Tomas Aparicio
+//
+//Permission is hereby granted, free of charge, to any person
+//obtaining a copy of this software and associated documentation
+//files (the "Software"), to deal in the Software without
+//restriction, including without limitation the rights to use,
+//copy, modify, merge, publish, distribute, sublicense, and/or sell
+//copies of the Software, and to permit persons to whom the
+//Software is furnished to do so, subject to the following
+//conditions:
+//
+//The above copyright notice and this permission notice shall be
+//included in all copies or substantial portions of the Software.
+//
+//THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+//EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+//OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+//NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+//HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+//WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+//FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+//OTHER DEALINGS IN THE SOFTWARE.
+
+package issvg
+
+import (
+	"regexp"
+	"unicode/utf8"
+)
+
+var (
+	// nolint:gosimple
+	htmlCommentRegex = regexp.MustCompile("(?i)<!--([\\s\\S]*?)-->")
+	svgRegex         = regexp.MustCompile(`(?i)^\s*(?:<\?xml[^>]*>\s*)?(?:<!doctype svg[^>]*>\s*)?<svg[^>]*>[^*]*<\/svg>\s*$`)
+)
+
+// isBinary checks if the given buffer is a binary file.
+func isBinary(buf []byte) bool {
+	if len(buf) < 24 {
+		return false
+	}
+	for i := 0; i < 24; i++ {
+		charCode, _ := utf8.DecodeRuneInString(string(buf[i]))
+		if charCode == 65533 || charCode <= 8 {
+			return true
+		}
+	}
+	return false
+}
+
+// Is returns true if the given buffer is a valid SVG image.
+func Is(buf []byte) bool {
+	return !isBinary(buf) && svgRegex.Match(htmlCommentRegex.ReplaceAll(buf, []byte{}))
+}
+
+// IsSVG returns true if the given buffer is a valid SVG image.
+// Alias to: Is()
+func IsSVG(buf []byte) bool {
+	return Is(buf)
+}

pkg/services/store/http.go

@@ -129,7 +129,6 @@ func (s *httpStorage) Upload(c *models.ReqContext) response.Response {
 
 			err = s.store.Upload(c.Req.Context(), c.SignedInUser, &UploadRequest{
 				Contents:              data,
-				MimeType:              mimeType,
 				EntityType:            entityType,
 				Path:                  path,
 				OverwriteExistingFile: overwriteExistingFile,

pkg/services/store/sanitize.go

@@ -2,6 +2,7 @@ package store
 
 import (
 	"context"
+	"mime"
 	"path/filepath"
 
 	"github.com/grafana/grafana/pkg/infra/filestorage"
@@ -41,10 +42,17 @@ func (s *standardStorageService) sanitizeUploadRequest(ctx context.Context, user
 		return nil, err
 	}
 
+	// we have already validated that the file contents match the extension in `./validate.go`
+	mimeType := mime.TypeByExtension(filepath.Ext(req.Path))
+	if mimeType == "" {
+		grafanaStorageLogger.Info("failed to find mime type", "path", req.Path)
+		mimeType = "application/octet-stream"
+	}
+
 	return &filestorage.UpsertFileCommand{
 		Path:               storagePath,
 		Contents:           contents,
-		MimeType:           req.MimeType,
+		MimeType:           mimeType,
 		CacheControl:       req.CacheControl,
 		ContentDisposition: req.ContentDisposition,
 		Properties:         req.Properties,

pkg/services/store/service.go

@@ -194,7 +194,9 @@ func ProvideService(sql *sqlstore.SQLStore, features featuremgmt.FeatureToggles,
 		}
 	})
 
-	return newStandardStorageService(sql, globalRoots, initializeOrgStorages, authService)
+	return newStandardStorageService(sql, globalRoots, initializeOrgStorages, authService, storageServiceConfig{
+		allowUnsanitizedSvgUpload: false,
+	})
 }
 
 func createSystemBrandingPathFilter() filestorage.PathFilter {
@@ -205,7 +207,7 @@ func createSystemBrandingPathFilter() filestorage.PathFilter {
 		nil)
 }
 
-func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRuntime, initializeOrgStorages func(orgId int64) []storageRuntime, authService storageAuthService) *standardStorageService {
+func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRuntime, initializeOrgStorages func(orgId int64) []storageRuntime, authService storageAuthService, cfg storageServiceConfig) *standardStorageService {
 	rootsByOrgId := make(map[int64][]storageRuntime)
 	rootsByOrgId[ac.GlobalOrgID] = globalRoots
 
@@ -218,9 +220,7 @@ func newStandardStorageService(sql *sqlstore.SQLStore, globalRoots []storageRunt
 		sql:         sql,
 		tree:        res,
 		authService: authService,
-		cfg: storageServiceConfig{
-			allowUnsanitizedSvgUpload: false,
-		},
+		cfg:         cfg,
 	}
 }
 
@@ -252,7 +252,6 @@ func (s *standardStorageService) Read(ctx context.Context, user *models.SignedIn
 
 type UploadRequest struct {
 	Contents           []byte
-	MimeType           string // TODO: remove MimeType from the struct once we can infer it from file contents
 	Path               string
 	CacheControl       string
 	ContentDisposition string
@@ -279,17 +278,17 @@ func (s *standardStorageService) Upload(ctx context.Context, user *models.Signed
 
 	validationResult := s.validateUploadRequest(ctx, user, req, storagePath)
 	if !validationResult.ok {
-		grafanaStorageLogger.Warn("file upload validation failed", "filetype", req.MimeType, "path", req.Path, "reason", validationResult.reason)
+		grafanaStorageLogger.Warn("file upload validation failed", "path", req.Path, "reason", validationResult.reason)
 		return ErrValidationFailed
 	}
 
 	upsertCommand, err := s.sanitizeUploadRequest(ctx, user, req, storagePath)
 	if err != nil {
-		grafanaStorageLogger.Error("failed while sanitizing the upload request", "filetype", req.MimeType, "path", req.Path, "error", err)
+		grafanaStorageLogger.Error("failed while sanitizing the upload request", "path", req.Path, "error", err)
 		return ErrUploadInternalError
 	}
 
-	grafanaStorageLogger.Info("uploading a file", "filetype", req.MimeType, "path", req.Path)
+	grafanaStorageLogger.Info("uploading a file", "path", req.Path)
 
 	if !req.OverwriteExistingFile {
 		file, err := root.Store().Get(ctx, storagePath)

pkg/services/store/service_test.go

@@ -3,6 +3,7 @@ package store
 import (
 	"bytes"
 	"context"
+	"io/ioutil"
 	"path/filepath"
 	"testing"
 
@@ -16,6 +17,9 @@ import (
 )
 
 var (
+	htmlBytes, _        = ioutil.ReadFile("testdata/page.html")
+	jpgBytes, _         = ioutil.ReadFile("testdata/image.jpg")
+	svgBytes, _         = ioutil.ReadFile("testdata/image.svg")
 	dummyUser           = &models.SignedInUser{OrgId: 1}
 	allowAllAuthService = newStaticStorageAuthService(func(ctx context.Context, user *models.SignedInUser, storageName string) map[string]filestorage.PathFilter {
 		return map[string]filestorage.PathFilter{
@@ -53,7 +57,7 @@ func TestListFiles(t *testing.T) {
 
 	store := newStandardStorageService(sqlstore.InitTestDB(t), roots, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
-	}, allowAllAuthService)
+	}, allowAllAuthService, storageServiceConfig{})
 	frame, err := store.List(context.Background(), dummyUser, "public/testdata")
 	require.NoError(t, err)
 
@@ -73,7 +77,7 @@ func TestListFilesWithoutPermissions(t *testing.T) {
 
 	store := newStandardStorageService(sqlstore.InitTestDB(t), roots, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
-	}, denyAllAuthService)
+	}, denyAllAuthService, storageServiceConfig{})
 	frame, err := store.List(context.Background(), dummyUser, "public/testdata")
 	require.NoError(t, err)
 	rowLen, err := frame.RowLen()
@@ -98,7 +102,7 @@ func setupUploadStore(t *testing.T, authService storageAuthService) (StorageServ
 	}
 	store := newStandardStorageService(sqlstore.InitTestDB(t), []storageRuntime{sqlStorage}, func(orgId int64) []storageRuntime {
 		return make([]storageRuntime, 0)
-	}, authService)
+	}, authService, storageServiceConfig{allowUnsanitizedSvgUpload: true})
 
 	return store, mockStorage, storageName
 }
@@ -106,14 +110,18 @@ func setupUploadStore(t *testing.T, authService storageAuthService) (StorageServ
 func TestShouldUploadWhenNoFileAlreadyExists(t *testing.T) {
 	service, mockStorage, storageName := setupUploadStore(t, nil)
 
-	mockStorage.On("Get", mock.Anything, "/myFile.jpg").Return(nil, nil)
-	mockStorage.On("Upsert", mock.Anything, mock.Anything).Return(nil)
+	fileName := "/myFile.jpg"
+	mockStorage.On("Get", mock.Anything, fileName).Return(nil, nil)
+	mockStorage.On("Upsert", mock.Anything, &filestorage.UpsertFileCommand{
+		Path:     fileName,
+		MimeType: "image/jpeg",
+		Contents: jpgBytes,
+	}).Return(nil)
 
 	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
 		EntityType: EntityTypeImage,
-		Contents:   make([]byte, 0),
-		Path:       storageName + "/myFile.jpg",
-		MimeType:   "image/jpg",
+		Contents:   jpgBytes,
+		Path:       storageName + fileName,
 	})
 	require.NoError(t, err)
 }
@@ -123,9 +131,8 @@ func TestShouldFailUploadWithoutAccess(t *testing.T) {
 
 	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
 		EntityType: EntityTypeImage,
-		Contents:   make([]byte, 0),
+		Contents:   jpgBytes,
 		Path:       storageName + "/myFile.jpg",
-		MimeType:   "image/jpg",
 	})
 	require.ErrorIs(t, err, ErrAccessDenied)
 }
@@ -137,9 +144,8 @@ func TestShouldFailUploadWhenFileAlreadyExists(t *testing.T) {
 
 	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
 		EntityType: EntityTypeImage,
-		Contents:   make([]byte, 0),
+		Contents:   jpgBytes,
 		Path:       storageName + "/myFile.jpg",
-		MimeType:   "image/jpg",
 	})
 	require.ErrorIs(t, err, ErrFileAlreadyExists)
 }
@@ -173,3 +179,50 @@ func TestShouldDelegateFolderDeletion(t *testing.T) {
 	})
 	require.NoError(t, err)
 }
+
+func TestShouldUploadSvg(t *testing.T) {
+	service, mockStorage, storageName := setupUploadStore(t, nil)
+
+	fileName := "/myFile.svg"
+	mockStorage.On("Get", mock.Anything, fileName).Return(nil, nil)
+	mockStorage.On("Upsert", mock.Anything, &filestorage.UpsertFileCommand{
+		Path:     fileName,
+		MimeType: "image/svg+xml",
+		Contents: svgBytes,
+	}).Return(nil)
+
+	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
+		EntityType: EntityTypeImage,
+		Contents:   svgBytes,
+		Path:       storageName + fileName,
+	})
+	require.NoError(t, err)
+}
+
+func TestShouldNotUploadHtmlDisguisedAsSvg(t *testing.T) {
+	service, mockStorage, storageName := setupUploadStore(t, nil)
+
+	fileName := "/myFile.svg"
+	mockStorage.On("Get", mock.Anything, fileName).Return(nil, nil)
+
+	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
+		EntityType: EntityTypeImage,
+		Contents:   htmlBytes,
+		Path:       storageName + fileName,
+	})
+	require.ErrorIs(t, err, ErrValidationFailed)
+}
+
+func TestShouldNotUploadJpgDisguisedAsSvg(t *testing.T) {
+	service, mockStorage, storageName := setupUploadStore(t, nil)
+
+	fileName := "/myFile.svg"
+	mockStorage.On("Get", mock.Anything, fileName).Return(nil, nil)
+
+	err := service.Upload(context.Background(), dummyUser, &UploadRequest{
+		EntityType: EntityTypeImage,
+		Contents:   jpgBytes,
+		Path:       storageName + fileName,
+	})
+	require.ErrorIs(t, err, ErrValidationFailed)
+}

pkg/services/store/testdata/image.svg

@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"><path d="M12,2A10,10,0,1,0,22,12,10,10,0,0,0,12,2Zm0,18a8,8,0,1,1,8-8A8,8,0,0,1,12,20Z"/></svg>

pkg/services/store/testdata/page.html

@@ -0,0 +1,9 @@
+<!DOCTYPE html>
+<html>
+<body>
+<h1>My First Heading</h1>
+
+<p>My first paragraph.</p>
+</body>
+</html>
+

pkg/services/store/validate.go

@@ -3,10 +3,14 @@ package store
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"net/http"
 	"path/filepath"
+	"strings"
 
 	"github.com/grafana/grafana/pkg/infra/filestorage"
 	"github.com/grafana/grafana/pkg/models"
+	issvg "github.com/grafana/grafana/pkg/services/store/go-is-svg"
 )
 
 var (
@@ -24,7 +28,7 @@ var (
 		".gif":  {"image/gif": true},
 		".png":  {"image/png": true},
 		".webp": {"image/webp": true},
-		".svg":  {"text/xml; charset=utf-8": true, "text/plain; charset=utf-8": true, "image/svg+xml": true},
+		".svg":  {"image/svg+xml": true},
 	}
 )
 
@@ -47,19 +51,24 @@ func fail(reason string) validationResult {
 }
 
 func (s *standardStorageService) detectMimeType(ctx context.Context, user *models.SignedInUser, uploadRequest *UploadRequest) string {
-	// TODO: implement a spoofing-proof MimeType detection based on the contents
-	return uploadRequest.MimeType
+	if strings.HasSuffix(uploadRequest.Path, ".svg") {
+		if issvg.IsSVG(uploadRequest.Contents) {
+			return "image/svg+xml"
+		}
+	}
+
+	return http.DetectContentType(uploadRequest.Contents)
 }
 
 func (s *standardStorageService) validateImage(ctx context.Context, user *models.SignedInUser, uploadRequest *UploadRequest) validationResult {
 	ext := filepath.Ext(uploadRequest.Path)
 	if !allowedImageExtensions[ext] {
-		return fail("unsupported extension")
+		return fail(fmt.Sprintf("unsupported extension: %s", ext))
 	}
 
 	mimeType := s.detectMimeType(ctx, user, uploadRequest)
 	if !imageExtensionsToMatchingMimeTypes[ext][mimeType] {
-		return fail("mismatched extension and file contents")
+		return fail(fmt.Sprintf("extension '%s' does not match the detected MimeType: %s", ext, mimeType))
 	}
 
 	return success()
@@ -70,7 +79,7 @@ func (s *standardStorageService) validateUploadRequest(ctx context.Context, user
 	// TODO: validateProperties
 
 	if err := filestorage.ValidatePath(storagePath); err != nil {
-		return fail("path validation failed. error:" + err.Error() + ". path: " + storagePath)
+		return fail(fmt.Sprintf("path validation failed. error: %s. path: %s", err.Error(), storagePath))
 	}
 
 	switch req.EntityType {