From 1154afebee986180b489084d38e2a0c578751498 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 30 Jun 2026 18:12:34 +0100 Subject: [PATCH] local: stop --links symlinks escaping the destination directory CVE-2026-54572 With -l/--links rclone recreates a .rclonelink object as a symlink. A malicious or compromised source could serve a symlink whose target points outside the destination, plus a sibling object whose path traverses it, so that rclone followed the planted symlink and wrote outside the destination causing arbitrary file write. When translating symlinks, rclone now performs all destination writes (directory creation, file writes and symlink creation) through an os.Root anchored at the destination. os.Root resolves every path component relative to the destination's file descriptor and refuses any that escapes the root, even under concurrent modification, so a planted symlink can never be traversed out of the destination. Symlinks are still reproduced verbatim - including ones whose target points outside the destination - so backups remain faithful. Only writing *through* such a link is refused. In-tree symlinks are unaffected. Fixes CVE-2026-54572 Fixes GHSA-cf44-9pgv-m4xc --- backend/local/local.go | 99 +++++++++++++++++--- backend/local/local_internal_test.go | 135 +++++++++++++++++++++++++++ docs/content/local.md | 25 +++++ 3 files changed, 244 insertions(+), 15 deletions(-) diff --git a/backend/local/local.go b/backend/local/local.go index 91ec9cdce..50bfba7d0 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -818,7 +818,7 @@ 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 := file.MkdirAll(localPath, 0777) + err := f.mkdirAll(localPath) if err != nil { return err } @@ -1435,10 +1435,84 @@ func (o *Object) Open(ctx context.Context, options ...fs.OpenOption) (in io.Read return in, nil } +// osRoot opens an *os.Root anchored at f.root, together with localPath +// expressed relative to it. The caller must Close the returned root. +// +// When translating symlinks (-l/--links) all writes go through an os.Root so a +// symlink planted by a malicious source can never be traversed to escape the +// destination (CWE-59, GHSA-cf44-9pgv-m4xc). +// +// f.root is the trusted destination the user chose, so it is created (and +// followed if it is itself a symlink) before being opened. +func (f *Fs) osRoot(localPath string) (root *os.Root, rel string, err error) { + rel, err = filepath.Rel(f.root, localPath) + if err != nil { + return nil, "", err + } + if err = file.MkdirAll(f.root, 0777); err != nil { + return nil, "", err + } + root, err = os.OpenRoot(f.root) + if err != nil { + return nil, "", err + } + return root, rel, nil +} + +// mkdirAll makes localPath and any missing parents. When translating +// symlinks it does so through os.Root so directory creation can't be +// redirected through a planted symlink out of the destination. +func (f *Fs) mkdirAll(localPath string) (err error) { + if !f.opt.TranslateSymlinks { + return file.MkdirAll(localPath, 0777) + } + root, rel, err := f.osRoot(localPath) + if err != nil { + return err + } + defer fs.CheckClose(root, &err) + if rel == "." { + return nil // the root itself, already created by linkRoot + } + return root.MkdirAll(rel, 0777) +} + +// openFile opens localPath for writing. When translating symlinks it goes +// through os.Root so a symlink planted at the path, or at any parent, is never +// followed out of the destination. The returned file is independent of the +// root, which is closed before returning. +func (f *Fs) openFile(localPath string, flags int, perm os.FileMode) (fi *os.File, err error) { + if !f.opt.TranslateSymlinks { + return file.OpenFile(localPath, flags, perm) + } + root, rel, err := f.osRoot(localPath) + if err != nil { + return nil, err + } + defer fs.CheckClose(root, &err) + return root.OpenFile(rel, flags, perm) +} + +// symlink creates a symlink with the given target at localPath, removing +// any existing file or symlink there first. It goes through os.Root, which +// creates the link verbatim (the target may point anywhere, preserving a +// faithful backup) but refuses to create it through a planted symlink, and +// won't remove a directory in the way. +func (f *Fs) symlink(target, localPath string) (err error) { + root, rel, err := f.osRoot(localPath) + if err != nil { + return err + } + defer fs.CheckClose(root, &err) + if err := root.Remove(rel); err != nil && !os.IsNotExist(err) { + return err + } + return root.Symlink(target, rel) +} + // mkdirAll makes all the directories needed to store the object func (o *Object) mkdirAll() error { - dir := filepath.Dir(o.path) - return file.MkdirAll(dir, 0777) + return o.fs.mkdirAll(filepath.Dir(o.path)) } type nopWriterCloser struct { @@ -1491,13 +1565,13 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op // If it is a translated link, just read in the contents, and // then create a symlink if !o.translatedLink { - f, err := file.OpenFile(o.path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) + f, err := o.fs.openFile(o.path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) if err != nil { if runtime.GOOS == "windows" && os.IsPermission(err) { // If permission denied on Windows might be trying to update a // hidden file, in which case try opening without CREATE // See: https://stackoverflow.com/questions/13215716/ioerror-errno-13-permission-denied-when-trying-to-open-hidden-file-in-w-mod - f, err = file.OpenFile(o.path, os.O_WRONLY|os.O_TRUNC, 0666) + f, err = o.fs.openFile(o.path, os.O_WRONLY|os.O_TRUNC, 0666) if err != nil { return err } @@ -1534,15 +1608,10 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op if o.translatedLink { if err == nil { - // Remove any current symlink or file, if one exists - if _, err := os.Lstat(o.path); err == nil { - if removeErr := os.Remove(o.path); removeErr != nil { - fs.Errorf(o, "Failed to remove previous file: %v", removeErr) - return removeErr - } - } - // Use the contents for the copied object to create a symlink - err = os.Symlink(symlinkData.String(), o.path) + // Use the contents of the copied object to create a symlink, + // without following or creating it through a planted symlink + // (CWE-59). Any existing file or symlink at the path is replaced. + err = o.fs.symlink(symlinkData.String(), o.path) } // only continue if symlink creation succeeded @@ -1606,7 +1675,7 @@ func (f *Fs) OpenWriterAt(ctx context.Context, remote string, size int64) (fs.Wr return nil, errors.New("can't open a symlink for random writing") } - out, err := file.OpenFile(o.path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) + out, err := f.openFile(o.path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666) if err != nil { return nil, err } diff --git a/backend/local/local_internal_test.go b/backend/local/local_internal_test.go index f36cc1791..1b178904d 100644 --- a/backend/local/local_internal_test.go +++ b/backend/local/local_internal_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "runtime" "sort" + "sync" "testing" "time" @@ -204,6 +205,140 @@ func TestSymlinkError(t *testing.T) { assert.Equal(t, errLinksAndCopyLinks, err) } +// putLink writes target as a translated link object (name + ".rclonelink") on f. +func putLink(ctx context.Context, f fs.Fs, name, target string) error { + in := bytes.NewBufferString(target) + src := object.NewStaticObjectInfo(name+fs.LinkSuffix, fstest.Time("2001-02-03T04:05:10Z"), int64(len(target)), true, nil, nil) + _, err := f.Put(ctx, in, src) + return err +} + +// putFile writes content as a regular object on f. +func putFile(ctx context.Context, f fs.Fs, remote, content string) error { + in := bytes.NewBufferString(content) + src := object.NewStaticObjectInfo(remote, fstest.Time("2001-02-03T04:05:10Z"), int64(len(content)), true, nil, nil) + _, err := f.Put(ctx, in, src) + return err +} + +// linksMode puts f into "-l/--links" mode, as if --links or the backend +// links=true option were set. +func linksMode(f *Fs) { + f.opt.FollowSymlinks = false + f.opt.TranslateSymlinks = true + f.lstat = os.Lstat +} + +// TestSymlinkEscapeWriteThroughBlocked mirrors the GHSA-cf44-9pgv-m4xc PoC: a +// malicious --links source serves "pwn.rclonelink" whose body is a path outside +// the destination, plus a sibling "pwn/authkeys" that sorts after it and would +// be written through the planted symlink. rclone reproduces the symlink (a +// faithful backup of the source) but must refuse to write through it, so +// nothing lands outside the destination (CWE-59). +func TestSymlinkEscapeWriteThroughBlocked(t *testing.T) { + ctx := context.Background() + + // A directory outside the destination the attacker wants to write into + evil := t.TempDir() + evilFile := filepath.Join(evil, "authkeys") + + r := fstest.NewRun(t) + f := r.Flocal.(*Fs) + linksMode(f) + + // The symlink is reproduced faithfully, pointing outside the destination. + require.NoError(t, putLink(ctx, f, "pwn", evil)) + link := filepath.Join(f.root, "pwn") + fi, err := os.Lstat(link) + require.NoError(t, err) + require.True(t, fi.Mode()&os.ModeSymlink != 0, "symlink should be reproduced faithfully") + target, err := os.Readlink(link) + require.NoError(t, err) + require.Equal(t, evil, target) + + // But writing the sibling object through it must be refused. + err = putFile(ctx, f, "pwn/authkeys", "PWNED") + require.Error(t, err, "writing through a planted symlink should be refused") + + // Nothing escaped the destination. + _, err = os.Stat(evilFile) + require.True(t, os.IsNotExist(err), "a file escaped the destination into %q", evilFile) +} + +// TestSymlinkEscapeNestedBlocked covers the chained variant: an in-tree symlink +// "evil" -> "." (the destination root) is created, then "evil/pwn" -> outside +// is planted through it, then a write nested under that. Every component is +// re-validated against the root, so the write-through is refused and nothing +// escapes. +func TestSymlinkEscapeNestedBlocked(t *testing.T) { + ctx := context.Background() + + evil := t.TempDir() + evilFile := filepath.Join(evil, "authkeys") + + r := fstest.NewRun(t) + f := r.Flocal.(*Fs) + linksMode(f) + + require.NoError(t, putLink(ctx, f, "evil", ".")) + require.NoError(t, putLink(ctx, f, "evil/pwn", evil)) + + err := putFile(ctx, f, "evil/pwn/authkeys", "PWNED") + require.Error(t, err, "writing through a nested planted symlink should be refused") + + _, err = os.Stat(evilFile) + require.True(t, os.IsNotExist(err), "a file escaped the destination into %q", evilFile) +} + +// TestSymlinkEscapeConcurrent races symlink creation against the sibling write +// for many pairs at once, exercising the time-of-check/time-of-use window. +// os.Root resolves relative to a directory file descriptor, so whatever the +// interleaving nothing may escape the destination. +func TestSymlinkEscapeConcurrent(t *testing.T) { + ctx := context.Background() + + evil := t.TempDir() + + r := fstest.NewRun(t) + f := r.Flocal.(*Fs) + linksMode(f) + + const pairs = 50 + var wg sync.WaitGroup + for i := range pairs { + name := fmt.Sprintf("pwn%d", i) + wg.Add(2) + go func() { defer wg.Done(); _ = putLink(ctx, f, name, evil) }() + go func() { defer wg.Done(); _ = putFile(ctx, f, name+"/authkeys", "PWNED") }() + } + wg.Wait() + + entries, err := os.ReadDir(evil) + require.NoError(t, err) + require.Empty(t, entries, "files escaped the destination into %q", evil) +} + +// TestSymlinkInTreeWriteThroughWorks checks the fix doesn't break legitimate +// use: an in-tree symlink to a sibling directory can still be created and +// written through, since that write stays inside the destination. +func TestSymlinkInTreeWriteThroughWorks(t *testing.T) { + ctx := context.Background() + + r := fstest.NewRun(t) + f := r.Flocal.(*Fs) + linksMode(f) + + require.NoError(t, putFile(ctx, f, "sub/keep.txt", "hello")) + require.NoError(t, putLink(ctx, f, "link", "sub")) + + require.NoError(t, putFile(ctx, f, "link/file.txt", "world")) + + // The write landed in the real sibling directory, inside the destination. + got, err := os.ReadFile(filepath.Join(f.root, "sub", "file.txt")) + require.NoError(t, err) + require.Equal(t, "world", string(got)) +} + func TestHashWithTypeNone(t *testing.T) { ctx := context.Background() r := fstest.NewRun(t) diff --git a/docs/content/local.md b/docs/content/local.md index f5c621efa..5f480dd19 100644 --- a/docs/content/local.md +++ b/docs/content/local.md @@ -296,6 +296,31 @@ backends and the VFS. Note that this flag is incompatible with `-copy-links` / `-L`. +#### Symlink targets and the destination + +When rclone recreates a `.rclonelink` file as a symlink on local storage, +the symlink can point anywhere - including, with an absolute path or one +using `../` - to a location outside the directory you are copying into. This +is normal - rclone reproduces whatever target the link had, so backups +round-trip faithfully. + +What rclone will **not** do is *write through* such a link. If a remote you +are copying from contains both a symlink and a file or directory that would +be placed inside it - for example a `dir.rclonelink` pointing somewhere +outside the destination, alongside a `dir/file.txt` - rclone refuses to +follow the symlink when writing `dir/file.txt`. The offending file is +skipped with an error, the rest of the transfer continues, and the skipped +file is counted in the error summary printed at the end of the run. + +This protects you from a malicious or compromised remote using `-l` / +`--links` to plant a symlink and then write through it to somewhere outside +your destination. Ordinary symlink round-trips, and symlinks that stay +inside the destination, are unaffected. + +If you have intentionally pre-created a symlinked directory inside your +destination and want rclone to write into the directory it points at, do +not use `-l` / `--links` for that copy, or remove the symlink first. + ### Restricting filesystems with --one-file-system Normally rclone will recurse through filesystems as mounted.