From 1a00ea7c6e06aecfe100c7ae4f4ad58c5e9e831b Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Sun, 11 Apr 2021 15:29:43 +0200 Subject: [PATCH] lib: Prevent using protocol method with native path (fixes #7557) (#7563) --- lib/fs/casefs.go | 3 +-- lib/fs/util.go | 18 +++++++++++++----- lib/model/folder.go | 2 +- lib/osutil/traversessymlink.go | 3 +-- lib/protocol/encryption.go | 16 +--------------- lib/protocol/encryption_test.go | 2 +- lib/scanner/virtualfs_test.go | 3 +-- lib/scanner/walk.go | 2 +- 8 files changed, 20 insertions(+), 29 deletions(-) diff --git a/lib/fs/casefs.go b/lib/fs/casefs.go index 0d9ab2500..1cdb0a7c1 100644 --- a/lib/fs/casefs.go +++ b/lib/fs/casefs.go @@ -11,7 +11,6 @@ import ( "errors" "fmt" "path/filepath" - "strings" "sync" "time" @@ -400,7 +399,7 @@ func (r *defaultRealCaser) realCase(name string) (string, error) { return realName, nil } - for _, comp := range strings.Split(name, string(PathSeparator)) { + for _, comp := range PathComponents(name) { node := r.cache.getExpireAdd(realName) node.once.Do(func() { diff --git a/lib/fs/util.go b/lib/fs/util.go index ee9969e18..348124dcf 100644 --- a/lib/fs/util.go +++ b/lib/fs/util.go @@ -15,6 +15,8 @@ import ( "unicode" ) +const pathSeparatorString = string(PathSeparator) + func ExpandTilde(path string) (string, error) { if path == "~" { return getHomeDir() @@ -164,7 +166,7 @@ func IsParent(path, parent string) bool { return path != "/" } if parent[len(parent)-1] != PathSeparator { - parent += string(PathSeparator) + parent += pathSeparatorString } return strings.HasPrefix(path, parent) } @@ -175,8 +177,8 @@ func CommonPrefix(first, second string) string { return "" } - firstParts := strings.Split(filepath.Clean(first), string(PathSeparator)) - secondParts := strings.Split(filepath.Clean(second), string(PathSeparator)) + firstParts := PathComponents(filepath.Clean(first)) + secondParts := PathComponents(filepath.Clean(second)) isAbs := filepath.IsAbs(first) && filepath.IsAbs(second) @@ -200,7 +202,7 @@ func CommonPrefix(first, second string) string { common = append(common, "") } else if len(common) == 1 { // If isAbs on non Windows, first element in both first and second is "", hence joining that returns nothing. - return string(PathSeparator) + return pathSeparatorString } } @@ -211,10 +213,16 @@ func CommonPrefix(first, second string) string { } // This has to be strings.Join, because filepath.Join([]string{"", "", "?", "C:", "Audrius"}...) returns garbage - result := strings.Join(common, string(PathSeparator)) + result := strings.Join(common, pathSeparatorString) return filepath.Clean(result) } +// PathComponents returns a list of names of parent directories and the leaf +// item for the given native (fs.PathSeparator delimited) and clean path. +func PathComponents(path string) []string { + return strings.Split(path, pathSeparatorString) +} + func isVolumeNameOnly(parts []string) bool { isNormalVolumeName := len(parts) == 1 && strings.HasSuffix(parts[0], ":") isUNCVolumeName := len(parts) == 4 && strings.HasSuffix(parts[3], ":") diff --git a/lib/model/folder.go b/lib/model/folder.go index 92045d4d6..bd3fdc1df 100644 --- a/lib/model/folder.go +++ b/lib/model/folder.go @@ -558,7 +558,7 @@ func (f *folder) scanSubdirsBatchAppendFunc(batch *fileInfoBatch) batchAppendFun // This is a "virtual" parent directory of encrypted files. // We don't track it, but check if anything still exists // within and delete it otherwise. - if fi.IsDirectory() && protocol.IsEncryptedParent(fi.Name) { + if fi.IsDirectory() && protocol.IsEncryptedParent(fs.PathComponents(fi.Name)) { if names, err := f.mtimefs.DirNames(fi.Name); err == nil && len(names) == 0 { f.mtimefs.Remove(fi.Name) } diff --git a/lib/osutil/traversessymlink.go b/lib/osutil/traversessymlink.go index 9764c3f36..bcb1d3577 100644 --- a/lib/osutil/traversessymlink.go +++ b/lib/osutil/traversessymlink.go @@ -9,7 +9,6 @@ package osutil import ( "fmt" "path/filepath" - "strings" "github.com/syncthing/syncthing/lib/fs" ) @@ -47,7 +46,7 @@ func TraversesSymlink(filesystem fs.Filesystem, name string) error { } var path string - for _, part := range strings.Split(name, string(fs.PathSeparator)) { + for _, part := range fs.PathComponents(name) { path = filepath.Join(path, part) info, err := filesystem.Lstat(path) if err != nil { diff --git a/lib/protocol/encryption.go b/lib/protocol/encryption.go index d89d27d49..b977d2e50 100644 --- a/lib/protocol/encryption.go +++ b/lib/protocol/encryption.go @@ -559,24 +559,10 @@ func (r rawResponse) Data() []byte { func (r rawResponse) Close() {} func (r rawResponse) Wait() {} -// IsEncryptedPath returns true if the path points at encrypted data. This is -// determined by checking for a sentinel string in the path. -func IsEncryptedPath(path string) bool { - pathComponents := strings.Split(path, "/") - if len(pathComponents) != 3 { - return false - } - return isEncryptedParentFromComponents(pathComponents[:2]) -} - // IsEncryptedParent returns true if the path points at a parent directory of // encrypted data, i.e. is not a "real" directory. This is determined by // checking for a sentinel string in the path. -func IsEncryptedParent(path string) bool { - return isEncryptedParentFromComponents(strings.Split(path, "/")) -} - -func isEncryptedParentFromComponents(pathComponents []string) bool { +func IsEncryptedParent(pathComponents []string) bool { l := len(pathComponents) if l == 2 && len(pathComponents[1]) != 2 { return false diff --git a/lib/protocol/encryption_test.go b/lib/protocol/encryption_test.go index b6fc2b555..63fcdd7c7 100644 --- a/lib/protocol/encryption_test.go +++ b/lib/protocol/encryption_test.go @@ -196,7 +196,7 @@ func TestIsEncryptedParent(t *testing.T) { {"1" + encryptedDirExtension + "/bc/" + comp + "/a/" + comp, false}, } for _, tc := range cases { - if res := IsEncryptedParent(tc.path); res != tc.is { + if res := IsEncryptedParent(strings.Split(tc.path, "/")); res != tc.is { t.Errorf("%v: got %v, expected %v", tc.path, res, tc.is) } } diff --git a/lib/scanner/virtualfs_test.go b/lib/scanner/virtualfs_test.go index 7179f5b53..365747565 100644 --- a/lib/scanner/virtualfs_test.go +++ b/lib/scanner/virtualfs_test.go @@ -10,7 +10,6 @@ import ( "errors" "fmt" "io" - "os" "path/filepath" "strings" "time" @@ -43,7 +42,7 @@ func (i infiniteFS) DirNames(name string) ([]string, error) { for j := 0; j < i.width; j++ { names = append(names, fmt.Sprintf("aa-file-%d", j)) } - if len(strings.Split(name, string(os.PathSeparator))) < i.depth { + if len(fs.PathComponents(name)) < i.depth { for j := 0; j < i.width; j++ { names = append(names, fmt.Sprintf("zz-dir-%d", j)) } diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go index 06080425a..795ab5fe0 100644 --- a/lib/scanner/walk.go +++ b/lib/scanner/walk.go @@ -311,7 +311,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, toHashChan chan<- protoco // ignored path need to be handled as well. // Prepend an empty string to handle ignoredParent without anything // appended in the first iteration. - for _, name := range append([]string{""}, strings.Split(rel, string(fs.PathSeparator))...) { + for _, name := range append([]string{""}, fs.PathComponents(rel)...) { ignoredParent = filepath.Join(ignoredParent, name) info, err = w.Filesystem.Lstat(ignoredParent) // An error here would be weird as we've already gotten to this point, but act on it nonetheless