Skip to content

Commit ef52a86

Browse files
ncwncw
committed
local: limit the size of symlink targets read from .rclonelink files
With -l/--links a .rclonelink object is buffered in memory to become the target of a symlink. The read was unbounded, so a hostile or corrupt source serving a large .rclonelink object made rclone use that much memory and then log the whole body in the resulting error. A symlink target can never be longer than a path, so the read now stops at 128 KiB and anything longer is refused without retrying.
1 parent 8cd4374 commit ef52a86

3 files changed

Lines changed: 37 additions & 0 deletions

File tree

‎backend/local/local.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ import (
3737
const (
3838
devUnset = 0xdeadbeefcafebabe // a device id meaning it is unset
3939
useReadDir = (runtime.GOOS == "windows" || runtime.GOOS == "plan9") // these OSes read FileInfos directly
40+
// maxLinkTargetSize is the largest symlink target accepted when
41+
// translating a .rclonelink object, comfortably above any OS path
42+
// limit (Windows allows 32767 UTF-16 units).
43+
maxLinkTargetSize = 128 * 1024
4044
)
4145

4246
// timeType allows the user to choose what exactly ModTime() returns
@@ -440,6 +444,7 @@ var (
440444
errLinksNeedsSuffix = errors.New("need \"" + fs.LinkSuffix + "\" suffix to refer to symlink when using -l/--links")
441445
errPathEscapes = errors.New("file name is not a path within the local root - check the encoding")
442446
errSymlinkLoop = errors.New("loop detected: points to a parent directory")
447+
errLinkTargetTooLong = errors.New("symlink target is too long to be a path")
443448
)
444449

445450
// NewFs constructs an Fs from the path
@@ -1736,6 +1741,9 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op
17361741
}
17371742
out = f
17381743
} else {
1744+
// The target is buffered in memory, so stop reading just past
1745+
// the longest acceptable one rather than trust the source
1746+
in = io.LimitReader(in, maxLinkTargetSize+1)
17391747
out = nopWriterCloser{&symlinkData}
17401748
}
17411749

@@ -1752,6 +1760,9 @@ func (o *Object) Update(ctx context.Context, in io.Reader, src fs.ObjectInfo, op
17521760
}
17531761

17541762
if o.translatedLink {
1763+
if err == nil && symlinkData.Len() > maxLinkTargetSize {
1764+
err = fserrors.NoRetryError(errLinkTargetTooLong)
1765+
}
17551766
if err == nil {
17561767
// Use the contents of the copied object to create a symlink,
17571768
// without following or creating it through a planted symlink

‎backend/local/local_internal_test.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,29 @@ func linksMode(f *Fs) {
272272
f.lstat = os.Lstat
273273
}
274274

275+
// TestSymlinkTargetTooLong checks that a .rclonelink object whose body is
276+
// far bigger than any path is refused without being buffered in memory,
277+
// and that nothing is created at the destination.
278+
func TestSymlinkTargetTooLong(t *testing.T) {
279+
skipIfNoSymlinks(t)
280+
ctx := context.Background()
281+
282+
r := fstest.NewRun(t)
283+
f := r.Flocal.(*Fs)
284+
linksMode(f)
285+
286+
// A source which never ends, so the read must be bounded
287+
src := object.NewStaticObjectInfo("big"+fs.LinkSuffix, fstest.Time("2001-02-03T04:05:10Z"), -1, true, nil, nil)
288+
in := readers.NewCountingReader(readers.NewPatternReader(1 << 40))
289+
_, err := f.Put(ctx, in, src)
290+
require.ErrorIs(t, err, errLinkTargetTooLong)
291+
assert.True(t, fserrors.IsNoRetryError(err))
292+
assert.LessOrEqual(t, in.BytesRead(), uint64(maxLinkTargetSize+1), "read more of the body than needed")
293+
294+
_, err = os.Lstat(filepath.Join(f.root, "big"))
295+
assert.True(t, os.IsNotExist(err), "nothing should be created for a refused symlink")
296+
}
297+
275298
// TestSymlinkEscapeWriteThroughBlocked mirrors the GHSA-cf44-9pgv-m4xc PoC: a
276299
// malicious --links source serves "pwn.rclonelink" whose body is a path outside
277300
// the destination, plus a sibling "pwn/authkeys" that sorts after it and would

‎docs/content/local.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,9 @@ $ tree /tmp/b
272272
└── file2 -> /home/user/file3
273273
```
274274

275+
A `.rclonelink` file whose contents are too long to be a path (more
276+
than 128 KiB) is refused rather than turned into a symlink.
277+
275278
However, if copied back without '-l'
276279

277280
```console

0 commit comments

Comments
 (0)