fs: confine directory listing entries that escape the root GHSA-3vxh-3pcx-9m8q GHSA-38xv-hf3p-h7mq CVE-PENDING
The rclone core does not sanitise ".." in an object's Remote(). Such a name can arrive from a malicious or buggy backend - an object store permits keys containing ".." or a leading "/" - and, if acted on, lets a listing or transfer escape the configured root. A source object named "../../other/x" is copied to "other/x" outside the destination root, and a crafted listing name surfaces outside the directory being listed. Add list.RemoteEscapesRoot, which reports whether a Remote climbs above the root when joined onto it, and list.RemoveEscaping, which drops and logs such entries. Apply RemoveEscaping unconditionally - independent of the include/exclude filters - at the three per-entry filtering points every listing passes through: filterDir, walk.listR and walk.walkRDirTree (recursive ListR). operations.StatJSON calls List and NewObject directly, bypassing those, so it rejects an escaping remote up front. This confines every backend at once, so no per-backend change is needed.
This commit is contained in:
@@ -305,6 +305,7 @@ func listR(ctx context.Context, f fs.Fs, path string, includeAll bool, listType
|
||||
}
|
||||
}
|
||||
listType.Filter(&entries)
|
||||
entries = list.RemoveEscaping(entries)
|
||||
if !includeAll {
|
||||
filteredEntries := entries[:0]
|
||||
for _, entry := range entries {
|
||||
@@ -473,6 +474,7 @@ func walkRDirTree(ctx context.Context, f fs.Fs, startPath string, includeAll boo
|
||||
var mu sync.Mutex
|
||||
err := listR(ctx, startPath, func(entries fs.DirEntries) error {
|
||||
accounting.Stats(ctx).Listed(int64(len(entries)))
|
||||
entries = list.RemoveEscaping(entries)
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
for _, entry := range entries {
|
||||
|
||||
@@ -788,6 +788,39 @@ func TestListType(t *testing.T) {
|
||||
assert.Equal(t, dirEntries, got)
|
||||
}
|
||||
|
||||
func TestListRConfinement(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
f, err := mockfs.NewFs(ctx, "mock", "/", nil)
|
||||
require.NoError(t, err)
|
||||
objects := fs.DirEntries{
|
||||
mockobject.Object("ok"),
|
||||
mockobject.Object(".."),
|
||||
mockobject.Object("../escape"),
|
||||
mockdir.New("../evildir"),
|
||||
// leading-slash climbers: filterDir doesn't guard the ListR path, so
|
||||
// these reach RemoveEscaping directly and must still be dropped.
|
||||
mockobject.Object("/../slashescape"),
|
||||
mockobject.Object("//../../slashescape2"),
|
||||
mockobject.Object("dir/deep"),
|
||||
}
|
||||
var got []string
|
||||
callback := func(entries fs.DirEntries) error {
|
||||
for _, entry := range entries {
|
||||
got = append(got, entry.Remote())
|
||||
}
|
||||
return nil
|
||||
}
|
||||
doListR := func(ctx context.Context, dir string, callback fs.ListRCallback) error {
|
||||
return callback(objects)
|
||||
}
|
||||
// includeAll = true exercises the unconditional confinement - the
|
||||
// include/exclude filter block is skipped in this mode, so an escaping
|
||||
// entry would otherwise pass straight through.
|
||||
err = listR(ctx, f, "", true, ListAll, callback, doListR, false)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, []string{"ok", "dir/deep"}, got)
|
||||
}
|
||||
|
||||
func TestListR(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
objects := fs.DirEntries{
|
||||
|
||||
Reference in New Issue
Block a user