diff --git a/backend/local/clone_darwin.go b/backend/local/clone_darwin.go index bf6e5445f..eec426d58 100644 --- a/backend/local/clone_darwin.go +++ b/backend/local/clone_darwin.go @@ -42,7 +42,10 @@ func (f *Fs) Copy(ctx context.Context, src fs.Object, remote string) (fs.Object, } // Create destination - dstObj := f.newObject(remote) + dstObj, err := f.newObject(remote) + if err != nil { + return nil, err + } err = dstObj.mkdirAll() if err != nil { return nil, err @@ -56,7 +59,11 @@ func (f *Fs) Copy(ctx context.Context, src fs.Object, remote string) (fs.Object, } } - err = Clone(srcPath, f.localPath(remote)) + dstPath, err := f.localPath(remote) + if err != nil { + return nil, err + } + err = Clone(srcPath, dstPath) if err != nil { return nil, err } diff --git a/backend/local/local.go b/backend/local/local.go index 6361c3922..cd0cc3890 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -437,6 +437,7 @@ type Directory struct { var ( errLinksAndCopyLinks = errors.New("can't use -l/--links with -L/--copy-links") errLinksNeedsSuffix = errors.New("need \"" + fs.LinkSuffix + "\" suffix to refer to symlink when using -l/--links") + errPathEscapes = errors.New("file name is not a path within the local root - check the encoding") ) // NewFs constructs an Fs from the path @@ -576,9 +577,12 @@ func translateLink(remote, localPath string) (newLocalPath string, isTranslatedL } // newObject makes a half completed Object -func (f *Fs) newObject(remote string) *Object { +func (f *Fs) newObject(remote string) (*Object, error) { translatedLink := false - localPath := f.localPath(remote) + localPath, err := f.localPath(remote) + if err != nil { + return nil, err + } if f.opt.TranslateSymlinks { // Possibly receive a new name for localPath @@ -590,14 +594,17 @@ func (f *Fs) newObject(remote string) *Object { remote: remote, path: localPath, translatedLink: translatedLink, - } + }, nil } // Return an Object from a path // // May return nil if an error occurred func (f *Fs) newObjectWithInfo(remote string, info os.FileInfo) (fs.Object, error) { - o := f.newObject(remote) + o, err := f.newObject(remote) + if err != nil { + return nil, err + } if info != nil { o.setMetadata(info) } else { @@ -630,12 +637,15 @@ func (f *Fs) NewObject(ctx context.Context, remote string) (fs.Object, error) { } // Create new directory object from the info passed in -func (f *Fs) newDirectory(dir string, fi os.FileInfo) *Directory { - o := f.newObject(dir) +func (f *Fs) newDirectory(dir string, fi os.FileInfo) (*Directory, error) { + o, err := f.newObject(dir) + if err != nil { + return nil, err + } o.setMetadata(fi) return &Directory{ Object: *o, - } + }, nil } // List the objects and directories in dir into entries. The @@ -650,7 +660,10 @@ func (f *Fs) newDirectory(dir string, fi os.FileInfo) *Directory { func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err error) { filter, useFilter := filter.GetConfig(ctx), filter.GetUseFilter(ctx) - fsDirPath := f.localPath(dir) + fsDirPath, err := f.localPath(dir) + if err != nil { + return nil, err + } _, err = os.Stat(fsDirPath) if err != nil { return nil, fs.ErrorDirNotFound @@ -752,7 +765,10 @@ func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err e // Ignore directories which are symlinks. These are junction points under windows which // are kind of a souped up symlink. Unix doesn't have directories which are symlinks. if (mode&symlinkFlag) == 0 && f.dev == readDevice(fi, f.opt.OneFileSystem) { - d := f.newDirectory(newRemote, fi) + d, err := f.newDirectory(newRemote, fi) + if err != nil { + return nil, err + } entries = append(entries, d) } } else { @@ -795,15 +811,29 @@ func (f *Fs) cleanRemote(dir, filename string) (remote string) { return } -func (f *Fs) localPath(name string) string { - return filepath.Join(f.root, filepath.FromSlash(f.opt.Enc.FromStandardPath(name))) +// localPath returns the OS path for the object called name, which is +// always underneath f.root. +// +// It returns errPathEscapes if name resolves outside f.root which can +// happen depending on the encoding. +func (f *Fs) localPath(name string) (string, error) { + native := filepath.FromSlash(f.opt.Enc.FromStandardPath(name)) + localPath := filepath.Join(f.root, native) + rel, err := filepath.Rel(f.root, localPath) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fserrors.NoRetryError(fmt.Errorf("%q: %w", name, errPathEscapes)) + } + return localPath, nil } // Put the Object to the local filesystem func (f *Fs) Put(ctx context.Context, in io.Reader, src fs.ObjectInfo, options ...fs.OpenOption) (fs.Object, error) { // Temporary Object under construction - info filled in by Update() - o := f.newObject(src.Remote()) - err := o.Update(ctx, in, src, options...) + o, err := f.newObject(src.Remote()) + if err != nil { + return nil, err + } + err = o.Update(ctx, in, src, options...) if err != nil { return nil, err } @@ -817,8 +847,11 @@ func (f *Fs) PutStream(ctx context.Context, in io.Reader, src fs.ObjectInfo, opt // Mkdir creates the directory if it doesn't exist func (f *Fs) Mkdir(ctx context.Context, dir string) error { - localPath := f.localPath(dir) - err := f.mkdirAll(localPath) + localPath, err := f.localPath(dir) + if err != nil { + return err + } + err = f.mkdirAll(localPath) if err != nil { return err } @@ -834,10 +867,14 @@ func (f *Fs) Mkdir(ctx context.Context, dir string) error { // DirSetModTime sets the directory modtime for dir func (f *Fs) DirSetModTime(ctx context.Context, dir string, modTime time.Time) error { + localPath, err := f.localPath(dir) + if err != nil { + return err + } o := Object{ fs: f, remote: dir, - path: f.localPath(dir), + path: localPath, } return o.SetModTime(ctx, modTime) } @@ -851,7 +888,10 @@ func (f *Fs) DirSetModTime(ctx context.Context, dir string, modTime time.Time) e // It returns the directory that was created. func (f *Fs) MkdirMetadata(ctx context.Context, dir string, metadata fs.Metadata) (fs.Directory, error) { // Find and or create the directory - localPath := f.localPath(dir) + localPath, err := f.localPath(dir) + if err != nil { + return nil, err + } fi, err := f.lstat(localPath) if errors.Is(err, os.ErrNotExist) { err := f.Mkdir(ctx, dir) @@ -867,7 +907,10 @@ func (f *Fs) MkdirMetadata(ctx context.Context, dir string, metadata fs.Metadata } // Create directory object - d := f.newDirectory(dir, fi) + d, err := f.newDirectory(dir, fi) + if err != nil { + return nil, err + } // Set metadata on the directory object if provided if metadata != nil { @@ -888,13 +931,16 @@ func (f *Fs) MkdirMetadata(ctx context.Context, dir string, metadata fs.Metadata // // If it isn't empty it will return an error func (f *Fs) Rmdir(ctx context.Context, dir string) error { - localPath := f.localPath(dir) + localPath, err := f.localPath(dir) + if err != nil { + return err + } if fi, err := os.Stat(localPath); err != nil { return err } else if !fi.IsDir() { return fs.ErrorIsFile } - err := os.Remove(localPath) + err = os.Remove(localPath) if runtime.GOOS == "windows" && errors.Is(err, iofs.ErrPermission) { // https://github.com/golang/go/issues/26295 if os.Chmod(localPath, 0o600) == nil { err = os.Remove(localPath) @@ -984,13 +1030,16 @@ func (f *Fs) Move(ctx context.Context, src fs.Object, remote string) (fs.Object, } // Temporary Object under construction - dstObj := f.newObject(remote) + dstObj, err := f.newObject(remote) + if err != nil { + return nil, err + } dstObj.fs.objectMetaMu.RLock() dstObjMode := dstObj.mode dstObj.fs.objectMetaMu.RUnlock() // Check it is a file if it exists - err := dstObj.lstat() + err = dstObj.lstat() if os.IsNotExist(err) { // OK } else if err != nil { @@ -1056,11 +1105,17 @@ func (f *Fs) DirMove(ctx context.Context, src fs.Fs, srcRemote, dstRemote string fs.Debugf(srcFs, "Can't move directory - not same remote type") return fs.ErrorCantDirMove } - srcPath := srcFs.localPath(srcRemote) - dstPath := f.localPath(dstRemote) + srcPath, err := srcFs.localPath(srcRemote) + if err != nil { + return err + } + dstPath, err := f.localPath(dstRemote) + if err != nil { + return err + } // Check if destination exists - _, err := os.Lstat(dstPath) + _, err = os.Lstat(dstPath) if !os.IsNotExist(err) { return fs.ErrorDirExists } @@ -1664,9 +1719,12 @@ var sparseWarning sync.Once // It truncates any existing object func (f *Fs) OpenWriterAt(ctx context.Context, remote string, size int64) (fs.WriterAtCloser, error) { // Temporary Object under construction - o := f.newObject(remote) + o, err := f.newObject(remote) + if err != nil { + return nil, err + } - err := o.mkdirAll() + err = o.mkdirAll() if err != nil { return nil, err } diff --git a/backend/local/local_internal_test.go b/backend/local/local_internal_test.go index 1b178904d..761aa8ea6 100644 --- a/backend/local/local_internal_test.go +++ b/backend/local/local_internal_test.go @@ -18,10 +18,12 @@ import ( "github.com/rclone/rclone/fs/accounting" "github.com/rclone/rclone/fs/config/configmap" "github.com/rclone/rclone/fs/filter" + "github.com/rclone/rclone/fs/fserrors" "github.com/rclone/rclone/fs/hash" "github.com/rclone/rclone/fs/object" "github.com/rclone/rclone/fs/operations" "github.com/rclone/rclone/fstest" + "github.com/rclone/rclone/lib/encoder" "github.com/rclone/rclone/lib/file" "github.com/rclone/rclone/lib/readers" "github.com/stretchr/testify/assert" @@ -339,6 +341,117 @@ func TestSymlinkInTreeWriteThroughWorks(t *testing.T) { require.Equal(t, "world", string(got)) } +// TestEncodingEscapeBlocked checks that a name from a malicious source can't +// be decoded into path syntax which writes outside the destination. +func TestEncodingEscapeBlocked(t *testing.T) { + ctx := context.Background() + outer := t.TempDir() + + // What a source backend which encodes Dot returns from Remote() for an + // object called "../marker.txt" - e.g. s3 listing the key + // "tenant/../marker.txt" with the remote rooted at "tenant". + remote := encoder.OS.ToStandardPath("../marker.txt") + require.Equal(t, "../marker.txt", remote) + + // A file outside the destination the attacker wants to overwrite. + marker := filepath.Join(outer, "marker.txt") + require.NoError(t, os.WriteFile(marker, []byte("original"), 0600)) + + // An encoding without Dot decodes the name back into "..". + fRaw, err := NewFs(ctx, "local", filepath.Join(outer, "dst"), configmap.Simple{"encoding": "Slash"}) + require.NoError(t, err) + f := fRaw.(*Fs) + + require.ErrorIs(t, putFile(ctx, f, remote, "PWNED"), errPathEscapes) + got, err := os.ReadFile(marker) + require.NoError(t, err) + require.Equal(t, "original", string(got), "a file outside the destination was overwritten") + + // The other entry points which resolve a name are refused too. + require.ErrorIs(t, f.Mkdir(ctx, remote), errPathEscapes) + require.ErrorIs(t, f.Rmdir(ctx, remote), errPathEscapes) + _, err = f.NewObject(ctx, remote) + require.ErrorIs(t, err, errPathEscapes) + + // Ordinary names in the same configuration still work. + require.NoError(t, putFile(ctx, f, "sub/file.txt", "hello")) + got, err = os.ReadFile(filepath.Join(f.root, "sub", "file.txt")) + require.NoError(t, err) + require.Equal(t, "hello", string(got)) + + // The entry points which take a name as a destination refuse it too. + src, err := f.NewObject(ctx, "sub/file.txt") + require.NoError(t, err) + _, err = f.Move(ctx, src, remote) + require.ErrorIs(t, err, errPathEscapes) + require.ErrorIs(t, f.DirMove(ctx, f, "sub", remote), errPathEscapes) + _, err = f.OpenWriterAt(ctx, remote, 5) + require.ErrorIs(t, err, errPathEscapes) + _, err = f.List(ctx, remote) + require.ErrorIs(t, err, errPathEscapes) + + // With the default encoding the name is stored literally as fullwidth + // dots, inside the destination. + dRaw, err := NewFs(ctx, "local", filepath.Join(outer, "default"), configmap.Simple{"encoding": encoder.OS.String()}) + require.NoError(t, err) + d := dRaw.(*Fs) + require.NoError(t, putFile(ctx, d, remote, "safe")) + got, err = os.ReadFile(filepath.Join(d.root, "..", "marker.txt")) + require.NoError(t, err) + require.Equal(t, "safe", string(got)) + + if runtime.GOOS == "windows" { + // A backslash is a path separator on Windows, so an encoding + // without BackSlash escapes the root even though it has Dot. + wRaw, err := NewFs(ctx, "local", filepath.Join(outer, "win"), configmap.Simple{"encoding": "Slash,Dot"}) + require.NoError(t, err) + require.ErrorIs(t, putFile(ctx, wRaw, `..\marker.txt`, "PWNED"), errPathEscapes) + got, err := os.ReadFile(marker) + require.NoError(t, err) + require.Equal(t, "original", string(got), "a file outside the destination was overwritten") + } +} + +// TestLocalPath checks which names localPath refuses. It must refuse +// exactly those which resolve outside the root - names which are merely +// unusual on the host platform are the OS's business, not ours. +func TestLocalPath(t *testing.T) { + ctx := context.Background() + fRaw, err := NewFs(ctx, "local", t.TempDir(), configmap.Simple{"encoding": "Raw"}) + require.NoError(t, err) + f := fRaw.(*Fs) + + const refused = "!" + for _, test := range []struct { + name string + want string // slash separated path relative to the root, or refused + }{ + {name: "", want: "."}, + {name: "file.txt", want: "file.txt"}, + {name: "sub/file.txt", want: "sub/file.txt"}, + {name: "sub/../file.txt", want: "file.txt"}, + {name: "..", want: refused}, + {name: "../marker.txt", want: refused}, + {name: "sub/../../marker.txt", want: refused}, + // Windows reserved device names are ordinary file names to + // rclone, which addresses the destination with \\?\ paths. + {name: "NUL", want: "NUL"}, + {name: "sub/aux.c", want: "sub/aux.c"}, + // An absolute name resolves relative to the root, as it always has. + {name: "/etc/passwd", want: "etc/passwd"}, + } { + got, err := f.localPath(test.name) + if test.want == refused { + assert.ErrorIs(t, err, errPathEscapes, test.name) + assert.True(t, fserrors.IsNoRetryError(err), test.name) + continue + } + if assert.NoError(t, err, test.name) { + assert.Equal(t, filepath.Join(f.root, filepath.FromSlash(test.want)), got, test.name) + } + } +} + func TestHashWithTypeNone(t *testing.T) { ctx := context.Background() r := fstest.NewRun(t)