Storage: validation and sanitization stubs (#50523)

* add `IsPathValidationError` util to fs api

* refactor storage.Upload method

* remove unused struct

* extract `RootUpload` constant

* move file validation outside of the service

* Make UploadErrorToStatusCode exported

* validation/sanitization

* refactor pathValidationError check

* refactor, rename sanitize to transform

* add a todo

* refactor

* transform -> sanitize

* lint fix

* #50608: fix jpg/jpeg

Co-authored-by: Tania B <yalyna.ts@gmail.com>
Co-authored-by: Ryan McKinley <ryantxu@gmail.com>

pkg/services/store/entity_events.go

@@ -25,6 +25,7 @@ type EntityType string
 const (
 	EntityTypeDashboard EntityType = "dashboard"
 	EntityTypeFolder    EntityType = "folder"
+	EntityTypeImage     EntityType = "image"
 )
 
 // CreateDatabaseEntityId creates entityId for entities stored in the existing SQL tables

pkg/services/store/http.go

@@ -34,18 +34,12 @@ func ProvideHTTPService(store StorageService) HTTPStorageService {
 func UploadErrorToStatusCode(err error) int {
 	switch {
 	case errors.Is(err, ErrUploadFeatureDisabled):
-		return 400
-
-	case errors.Is(err, ErrUnsupportedFolder):
-		return 400
-
-	case errors.Is(err, ErrFileTooBig):
-		return 400
+		return 404
 
-	case errors.Is(err, ErrInvalidPath):
+	case errors.Is(err, ErrUnsupportedStorage):
 		return 400
 
-	case errors.Is(err, ErrInvalidFileType):
+	case errors.Is(err, ErrValidationFailed):
 		return 400
 
 	case errors.Is(err, ErrFileAlreadyExists):
@@ -102,9 +96,10 @@ func (s *httpStorage) Upload(c *models.ReqContext) response.Response {
 
 	mimeType := http.DetectContentType(data)
 
-	err = s.store.Upload(c.Req.Context(), c.SignedInUser, UploadRequest{
+	err = s.store.Upload(c.Req.Context(), c.SignedInUser, &UploadRequest{
 		Contents:              data,
 		MimeType:              mimeType,
+		EntityType:            EntityTypeImage,
 		Path:                  path,
 		OverwriteExistingFile: true,
 	})

pkg/services/store/sanitize.go

@@ -0,0 +1,28 @@
+package store
+
+import (
+	"context"
+	"path/filepath"
+
+	"github.com/grafana/grafana/pkg/infra/filestorage"
+	"github.com/grafana/grafana/pkg/models"
+)
+
+func (s *standardStorageService) sanitizeUploadRequest(ctx context.Context, user *models.SignedInUser, req *UploadRequest, storagePath string) (*filestorage.UpsertFileCommand, error) {
+	if req.EntityType == EntityTypeImage {
+		ext := filepath.Ext(req.Path)
+		//nolint: staticcheck
+		if ext == ".svg" {
+			// TODO: sanitize svg
+		}
+	}
+
+	return &filestorage.UpsertFileCommand{
+		Path:               storagePath,
+		Contents:           req.Contents,
+		MimeType:           req.MimeType,
+		CacheControl:       req.CacheControl,
+		ContentDisposition: req.ContentDisposition,
+		Properties:         req.Properties,
+	}, nil
+}

pkg/services/store/service.go

@@ -20,11 +20,9 @@ import (
 var grafanaStorageLogger = log.New("grafanaStorageLogger")
 
 var ErrUploadFeatureDisabled = errors.New("upload feature is disabled")
-var ErrUnsupportedFolder = errors.New("unsupported folder for uploads")
-var ErrFileTooBig = errors.New("file is too big")
-var ErrInvalidPath = errors.New("path is invalid")
+var ErrUnsupportedStorage = errors.New("storage does not support upload operation")
 var ErrUploadInternalError = errors.New("upload internal error")
-var ErrInvalidFileType = errors.New("invalid file type")
+var ErrValidationFailed = errors.New("request validation failed")
 var ErrFileAlreadyExists = errors.New("file exists")
 
 const RootPublicStatic = "public-static"
@@ -41,9 +39,14 @@ type StorageService interface {
 	// Read raw file contents out of the store
 	Read(ctx context.Context, user *models.SignedInUser, path string) (*filestorage.File, error)
 
-	Upload(ctx context.Context, user *models.SignedInUser, req UploadRequest) error
+	Upload(ctx context.Context, user *models.SignedInUser, req *UploadRequest) error
 
 	Delete(ctx context.Context, user *models.SignedInUser, path string) error
+
+	validateUploadRequest(ctx context.Context, user *models.SignedInUser, req *UploadRequest, storagePath string) validationResult
+
+	// sanitizeUploadRequest sanitizes the upload request and converts it into a command accepted by the FileStorage API
+	sanitizeUploadRequest(ctx context.Context, user *models.SignedInUser, req *UploadRequest, storagePath string) (*filestorage.UpsertFileCommand, error)
 }
 
 type standardStorageService struct {
@@ -117,48 +120,43 @@ func (s *standardStorageService) Read(ctx context.Context, user *models.SignedIn
 	return s.tree.GetFile(ctx, getOrgId(user), path)
 }
 
-func isFileTypeValid(filetype string) bool {
-	if (filetype == "image/jpeg") || (filetype == "image/jpg") || (filetype == "image/gif") || (filetype == "image/png") || (filetype == "image/webp") {
-		return true
-	}
-	return false
-}
-
 type UploadRequest struct {
 	Contents           []byte
-	MimeType           string
+	MimeType           string // TODO: remove MimeType from the struct once we can infer it from file contents
 	Path               string
 	CacheControl       string
 	ContentDisposition string
 	Properties         map[string]string
+	EntityType         EntityType
 
 	OverwriteExistingFile bool
 }
 
-func (s *standardStorageService) Upload(ctx context.Context, user *models.SignedInUser, req UploadRequest) error {
+func (s *standardStorageService) Upload(ctx context.Context, user *models.SignedInUser, req *UploadRequest) error {
 	upload, _ := s.tree.getRoot(getOrgId(user), RootUpload)
 	if upload == nil {
 		return ErrUploadFeatureDisabled
 	}
 
 	if !strings.HasPrefix(req.Path, RootUpload+"/") {
-		return ErrUnsupportedFolder
-	}
-
-	validFileType := isFileTypeValid(req.MimeType)
-	if !validFileType {
-		return ErrInvalidFileType
+		return ErrUnsupportedStorage
 	}
 
-	grafanaStorageLogger.Info("uploading a file", "filetype", req.MimeType, "path", req.Path)
-
 	storagePath := strings.TrimPrefix(req.Path, RootUpload)
+	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)
+		return ErrValidationFailed
+	}
 
-	if err := filestorage.ValidatePath(storagePath); err != nil {
-		grafanaStorageLogger.Info("uploading file failed due to invalid path", "filetype", req.MimeType, "path", req.Path, "err", err)
-		return ErrInvalidPath
+	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)
+		return ErrUploadInternalError
 	}
 
+	grafanaStorageLogger.Info("uploading a file", "filetype", req.MimeType, "path", req.Path)
+
 	if !req.OverwriteExistingFile {
 		file, err := upload.Get(ctx, storagePath)
 		if err != nil {
@@ -171,16 +169,7 @@ func (s *standardStorageService) Upload(ctx context.Context, user *models.Signed
 		}
 	}
 
-	err := upload.Upsert(ctx, &filestorage.UpsertFileCommand{
-		Path:               storagePath,
-		Contents:           req.Contents,
-		MimeType:           req.MimeType,
-		CacheControl:       req.CacheControl,
-		ContentDisposition: req.ContentDisposition,
-		Properties:         req.Properties,
-	})
-
-	if err != nil {
+	if err := upload.Upsert(ctx, upsertCommand); err != nil {
 		grafanaStorageLogger.Error("failed while uploading the file", "err", err, "path", req.Path)
 		return ErrUploadInternalError
 	}

pkg/services/store/service_test.go

@@ -61,10 +61,11 @@ func TestUpload(t *testing.T) {
 	cfg := &setting.Cfg{AppURL: "http://localhost:3000/", DataPath: path}
 	s := ProvideService(sqlstore.InitTestDB(t), features, cfg)
 	request := UploadRequest{
-		Contents: make([]byte, 0),
-		Path:     "upload/myFile.jpg",
-		MimeType: "image/jpeg",
+		EntityType: EntityTypeImage,
+		Contents:   make([]byte, 0),
+		Path:       "upload/myFile.jpg",
+		MimeType:   "image/jpg",
 	}
-	err = s.Upload(context.Background(), dummyUser, request)
+	err = s.Upload(context.Background(), dummyUser, &request)
 	require.NoError(t, err)
 }

pkg/services/store/validate.go

@@ -0,0 +1,90 @@
+package store
+
+import (
+	"context"
+	"encoding/json"
+	"path/filepath"
+
+	"github.com/grafana/grafana/pkg/infra/filestorage"
+	"github.com/grafana/grafana/pkg/models"
+)
+
+var (
+	allowedImageExtensions = map[string]bool{
+		".jpg":  true,
+		".jpeg": true,
+		".gif":  true,
+		".png":  true,
+		".webp": true,
+	}
+	imageExtensionsToMatchingMimeTypes = map[string]map[string]bool{
+		".jpg":  {"image/jpg": true, "image/jpeg": true},
+		".jpeg": {"image/jpg": true, "image/jpeg": true},
+		".gif":  {"image/gif": true},
+		".png":  {"image/png": true},
+		".webp": {"image/webp": true},
+	}
+)
+
+type validationResult struct {
+	ok     bool
+	reason string
+}
+
+func success() validationResult {
+	return validationResult{
+		ok: true,
+	}
+}
+
+func fail(reason string) validationResult {
+	return validationResult{
+		ok:     false,
+		reason: reason,
+	}
+}
+
+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
+}
+
+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")
+	}
+
+	mimeType := s.detectMimeType(ctx, user, uploadRequest)
+	if !imageExtensionsToMatchingMimeTypes[ext][mimeType] {
+		return fail("mismatched extension and file contents")
+	}
+
+	return success()
+}
+
+func (s *standardStorageService) validateUploadRequest(ctx context.Context, user *models.SignedInUser, req *UploadRequest, storagePath string) validationResult {
+	// TODO: validateSize
+	// TODO: validateProperties
+
+	if err := filestorage.ValidatePath(storagePath); err != nil {
+		return fail("path validation failed: " + err.Error())
+	}
+
+	switch req.EntityType {
+	case EntityTypeFolder:
+		fallthrough
+	case EntityTypeDashboard:
+		// TODO: add proper validation
+		var something interface{}
+		if err := json.Unmarshal(req.Contents, &something); err != nil {
+			return fail(err.Error())
+		}
+
+		return success()
+	case EntityTypeImage:
+		return s.validateImage(ctx, user, req)
+	default:
+		return fail("unknown entity")
+	}
+}