diff --git a/pkg/infra/filestorage/api.go b/pkg/infra/filestorage/api.go index 730186fad2c..645ae462cf9 100644 --- a/pkg/infra/filestorage/api.go +++ b/pkg/infra/filestorage/api.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "path/filepath" "regexp" "strings" "time" @@ -13,13 +14,57 @@ var ( ErrRelativePath = errors.New("path cant be relative") ErrNonCanonicalPath = errors.New("path must be canonical") ErrPathTooLong = errors.New("path is too long") - ErrPathInvalid = errors.New("path is invalid") + ErrInvalidCharacters = errors.New("path contains unsupported characters") ErrPathEndsWithDelimiter = errors.New("path can not end with delimiter") + ErrPathPartTooLong = errors.New("path part is too long") + ErrEmptyPathPart = errors.New("path can not have empty parts") Delimiter = "/" DirectoryMimeType = "directory" multipleDelimiters = regexp.MustCompile(`/+`) + pathRegex = regexp.MustCompile(`(^/$)|(^(/[A-Za-z\d!\-_.*'() ]+)+$)`) + maxPathLength = 1024 + maxPathPartLength = 256 ) +func ValidatePath(path string) error { + if !filepath.IsAbs(path) { + return ErrRelativePath + } + + if path == Delimiter { + return nil + } + + if strings.HasSuffix(path, Delimiter) { + return ErrPathEndsWithDelimiter + } + + if filepath.Clean(path) != path { + return ErrNonCanonicalPath + } + + if len(path) > maxPathLength { + return ErrPathTooLong + } + + for _, part := range strings.Split(strings.TrimPrefix(path, Delimiter), Delimiter) { + if strings.TrimSpace(part) == "" { + return ErrEmptyPathPart + } + + if len(part) > maxPathPartLength { + return ErrPathPartTooLong + } + } + + matches := pathRegex.MatchString(path) + if !matches { + return ErrInvalidCharacters + } + + return nil +} + func Join(parts ...string) string { joinedPath := Delimiter + strings.Join(parts, Delimiter) diff --git a/pkg/infra/filestorage/api_test.go b/pkg/infra/filestorage/api_test.go index ca557e3a140..f22393bdd36 100644 --- a/pkg/infra/filestorage/api_test.go +++ b/pkg/infra/filestorage/api_test.go @@ -1,6 +1,7 @@ package filestorage import ( + "strings" "testing" "github.com/stretchr/testify/require" @@ -34,3 +35,81 @@ func TestFilestorageApi_Join(t *testing.T) { }) } } + +func pathPart(length int) string { + sb := strings.Builder{} + for i := 0; i < length; i++ { + sb.WriteString("a") + } + return sb.String() +} + +func TestFilestorageApi_ValidatePath(t *testing.T) { + var tests = []struct { + path string + expectedError error + }{ + { + path: "abc/file.jpg", + expectedError: ErrRelativePath, + }, + { + path: "../abc/file.jpg", + expectedError: ErrRelativePath, + }, + { + path: "/abc/./file.jpg", + expectedError: ErrNonCanonicalPath, + }, + { + path: "/abc/../abc/file.jpg", + expectedError: ErrNonCanonicalPath, + }, + { + path: "/abc//folder/file.jpg", + expectedError: ErrNonCanonicalPath, + }, + { + path: "/abc/file.jpg/", + expectedError: ErrPathEndsWithDelimiter, + }, + { + path: "/abc/folder/ ", + expectedError: ErrEmptyPathPart, + }, + { + path: "/abc/ /file.jpg", + expectedError: ErrEmptyPathPart, + }, + { + path: "/abc/" + pathPart(260) + "/file.jpg", + expectedError: ErrPathPartTooLong, + }, + { + path: "/abc/" + pathPart(1050) + "/file.jpg", + expectedError: ErrPathTooLong, + }, + { + path: "/abc/folder🚀/file.jpg", + expectedError: ErrInvalidCharacters, + }, + { + path: "/path/with/utf/char/at/the/end.jpg�", + expectedError: ErrInvalidCharacters, + }, + { + path: "/myFile/file.jpg", + }, + } + for _, tt := range tests { + if tt.expectedError == nil { + t.Run("path "+tt.path+" should be valid", func(t *testing.T) { + require.Nil(t, ValidatePath(tt.path)) + }) + } else { + t.Run("path "+tt.path+" should be invalid: "+tt.expectedError.Error(), func(t *testing.T) { + require.Equal(t, tt.expectedError, ValidatePath(tt.path)) + }) + } + } +} diff --git a/pkg/infra/filestorage/wrapper.go b/pkg/infra/filestorage/wrapper.go index 978777f3e7e..4165053dc74 100644 --- a/pkg/infra/filestorage/wrapper.go +++ b/pkg/infra/filestorage/wrapper.go @@ -5,7 +5,6 @@ import ( "fmt" "mime" "path/filepath" - "regexp" "strings" "github.com/grafana/grafana/pkg/infra/log" @@ -14,7 +13,6 @@ import ( var ( directoryMarker = ".___gf_dir_marker___" - pathRegex = regexp.MustCompile(`(^/$)|(^(/[A-Za-z0-9!\-_.*'() ]+)+$)`) ) type wrapper struct { @@ -100,37 +98,8 @@ func getName(path string) string { return split[len(split)-1] } -func validatePath(path string) error { - if !filepath.IsAbs(path) { - return ErrRelativePath - } - - if path == Delimiter { - return nil - } - - if filepath.Clean(path) != path { - return ErrNonCanonicalPath - } - - if strings.HasSuffix(path, Delimiter) { - return ErrPathEndsWithDelimiter - } - - if len(path) > 1000 { - return ErrPathTooLong - } - - matches := pathRegex.MatchString(path) - if !matches { - return ErrPathInvalid - } - - return nil -} - func (b wrapper) validatePath(path string) error { - if err := validatePath(path); err != nil { + if err := ValidatePath(path); err != nil { b.log.Error("Path failed validation", "path", path, "error", err) return err }