archive: reject unsafe entry names when mounting squashfs images GHSA-66hp-wgxq-6f5q
Entry names read from a squashfs directory are not sanitized by go-diskfs. The squashfs backend joined each leaf name onto its directory to form the object's remote, so a crafted image could escape its directory. Use sanitize.Leaf to skip unsafe entries in List. A "\" is an ordinary character in a file name on the systems squashfs images are made on and in an rclone remote path, so it is deliberately not rejected; making it safe for the destination is the destination backend's job. Skipped entries are logged at DEBUG with a single NOTICE count per listing so a crafted image under a mount cannot flood the log.
This commit is contained in:
@@ -16,6 +16,7 @@ import (
|
||||
"github.com/rclone/rclone/fs/hash"
|
||||
"github.com/rclone/rclone/fs/log"
|
||||
"github.com/rclone/rclone/lib/readers"
|
||||
"github.com/rclone/rclone/lib/sanitize"
|
||||
"github.com/rclone/rclone/vfs"
|
||||
"github.com/rclone/rclone/vfs/vfscommon"
|
||||
)
|
||||
@@ -237,8 +238,15 @@ func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err e
|
||||
}
|
||||
|
||||
entries = make(fs.DirEntries, 0, len(items))
|
||||
skipped := 0
|
||||
for _, item := range items {
|
||||
// fs.Debugf(item.Name(), "entry = %#v", item)
|
||||
// Skip entry names that aren't safe
|
||||
if err := sanitize.Leaf(item.Name()); err != nil {
|
||||
fs.Debugf(f, "Skipping squashfs entry %q which escapes the archive", item.Name())
|
||||
skipped++
|
||||
continue
|
||||
}
|
||||
var entry fs.DirEntry
|
||||
if item.IsDir() {
|
||||
remote := f.fromNative(nativeDir, item.Name())
|
||||
@@ -260,6 +268,12 @@ func (f *Fs) List(ctx context.Context, dir string) (entries fs.DirEntries, err e
|
||||
entries = append(entries, entry)
|
||||
}
|
||||
|
||||
// Log once per listing rather than per entry so a crafted image
|
||||
// can't flood the log
|
||||
if skipped > 0 {
|
||||
fs.Logf(f, "Skipped %d squashfs entries in %q whose names escape the archive", skipped, dir)
|
||||
}
|
||||
|
||||
// fs.Debugf(f, "dir=%q, entries=%v", dir, entries)
|
||||
return entries, nil
|
||||
}
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
package squashfs
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
_ "github.com/rclone/rclone/backend/local"
|
||||
fscache "github.com/rclone/rclone/fs/cache"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// A backslash is an ordinary character in a file name on the systems
|
||||
// squashfs images are made on, so an entry containing one must be
|
||||
// listed and openable like any other file.
|
||||
func TestBackslashInName(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
localFs, err := fscache.Get(ctx, "testdata")
|
||||
require.NoError(t, err)
|
||||
|
||||
f, err := New(ctx, localFs, "backslash.sqfs", "", "")
|
||||
require.NoError(t, err)
|
||||
|
||||
entries, err := f.List(ctx, "")
|
||||
require.NoError(t, err)
|
||||
require.Len(t, entries, 1)
|
||||
assert.Equal(t, `back\slash.txt`, entries[0].Remote())
|
||||
|
||||
o, err := f.NewObject(ctx, `back\slash.txt`)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, int64(6), o.Size())
|
||||
}
|
||||
Binary file not shown.
Reference in New Issue
Block a user