local: don't restore setuid/setgid/sticky bits from metadata by default GHSA-945v-v9p3-v5xw
When applying the "mode" from --metadata the local backend cast the source value straight to an os.FileMode, so a source that supplied a mode with Go's setuid, setgid or sticky bits set would have those bits applied to the freshly written file. As both the file content and its metadata come from the source remote, a malicious source could plant a setuid binary, and a victim running "rclone copy -M" as root against an untrusted remote could end up with a root-owned setuid binary with attacker-controlled content. Rclone records "mode" in the unix st_mode layout where the special bits live in different positions to Go's os.FileMode, so honest sources never actually round-tripped these bits in the first place. Apply only the permission bits by default, which closes this off and is backwards compatible, and add the --local-metadata-restore-special-bits lag to restore the previous behaviour for trusted sources such as restoring a system backup made by rclone. See: GHSA-945v-v9p3-v5xw
This commit is contained in:
@@ -78,6 +78,13 @@ User metadata is stored as extended attributes (which may not be
|
||||
supported by all file systems) under the "user.*" prefix.
|
||||
|
||||
Metadata is supported on files and directories.
|
||||
|
||||
When restoring metadata with ` + "`--metadata`" + ` rclone applies the
|
||||
"mode", "uid" and "gid" from the source. These come from the source
|
||||
remote which may not be trusted, so restoring metadata as root from an
|
||||
untrusted source can change file ownership and is not recommended. The
|
||||
setuid, setgid and sticky bits are not restored by default - see the
|
||||
` + "`--local-metadata-restore-special-bits`" + ` flag.
|
||||
`,
|
||||
},
|
||||
Options: []fs.Option{
|
||||
@@ -280,6 +287,26 @@ enabled, rclone will no longer update the modtime after copying a file.`,
|
||||
Default: false,
|
||||
Advanced: true,
|
||||
},
|
||||
{
|
||||
Name: "metadata_restore_special_bits",
|
||||
Help: `Restore the setuid, setgid and sticky bits from metadata.
|
||||
|
||||
When restoring metadata with --metadata rclone applies the "mode" from
|
||||
the source. By default rclone applies only the permission bits and
|
||||
strips the setuid, setgid and sticky bits.
|
||||
|
||||
The "mode" comes from the source remote which may not be trusted.
|
||||
Restoring a setuid or setgid bit onto freshly written,
|
||||
source-controlled content can plant a setuid binary, which is dangerous
|
||||
in particular when restoring from an untrusted source while running as
|
||||
root. For this reason these bits are not restored by default.
|
||||
|
||||
If you trust the source and want the setuid, setgid and sticky bits
|
||||
restored - for example when restoring a system backup made by rclone -
|
||||
set this flag.`,
|
||||
Default: false,
|
||||
Advanced: true,
|
||||
},
|
||||
{
|
||||
Name: "fatal_if_no_space",
|
||||
Help: `Make out-of-space errors fatal during transfers.
|
||||
@@ -365,6 +392,7 @@ type Options struct {
|
||||
Hashes fs.CommaSepList `config:"hashes"`
|
||||
Enc encoder.MultiEncoder `config:"encoding"`
|
||||
NoClone bool `config:"no_clone"`
|
||||
MetadataRestoreSpecial bool `config:"metadata_restore_special_bits"`
|
||||
}
|
||||
|
||||
// Fs represents a local filesystem rooted at root
|
||||
|
||||
@@ -471,6 +471,59 @@ func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) {
|
||||
})
|
||||
}
|
||||
|
||||
// Check that the setuid, setgid and sticky bits from "mode" metadata are
|
||||
// stripped by default and only restored with --local-metadata-restore-special-bits.
|
||||
//
|
||||
// See: https://github.com/rclone/rclone/security/advisories/GHSA-945v-v9p3-v5xw
|
||||
func TestMetadataSpecialBits(t *testing.T) {
|
||||
switch runtime.GOOS {
|
||||
case "windows", "plan9", "js":
|
||||
t.Skip("mode metadata is not applied on this OS")
|
||||
}
|
||||
ctx := context.Background()
|
||||
r := fstest.NewRun(t)
|
||||
const filePath = "setuid.bin"
|
||||
r.WriteFile(filePath, "payload", time.Now())
|
||||
f := r.Flocal.(*Fs)
|
||||
|
||||
obj, err := f.NewObject(ctx, filePath)
|
||||
require.NoError(t, err)
|
||||
o := obj.(*Object)
|
||||
osPath := filepath.Join(f.root, filePath)
|
||||
|
||||
statMode := func() os.FileMode {
|
||||
t.Helper()
|
||||
fi, err := os.Stat(osPath)
|
||||
require.NoError(t, err)
|
||||
return fi.Mode()
|
||||
}
|
||||
|
||||
// "40000755" is Go's os.FileMode layout for setuid|0755 - a value a real
|
||||
// unix st_mode can never produce, so it can only come from an
|
||||
// attacker-controlled source remote.
|
||||
const setuidMode = "40000755"
|
||||
|
||||
t.Run("StrippedByDefault", func(t *testing.T) {
|
||||
require.NoError(t, os.Chmod(osPath, 0644))
|
||||
require.NoError(t, o.writeMetadataToFile(fs.Metadata{"mode": setuidMode}))
|
||||
mode := statMode()
|
||||
assert.Equal(t, os.FileMode(0755), mode.Perm())
|
||||
assert.Zero(t, mode&(os.ModeSetuid|os.ModeSetgid|os.ModeSticky),
|
||||
fmt.Sprintf("special bits should be stripped by default, got %v", mode))
|
||||
})
|
||||
|
||||
t.Run("RestoredWithFlag", func(t *testing.T) {
|
||||
f.opt.MetadataRestoreSpecial = true
|
||||
defer func() { f.opt.MetadataRestoreSpecial = false }()
|
||||
require.NoError(t, os.Chmod(osPath, 0644))
|
||||
require.NoError(t, o.writeMetadataToFile(fs.Metadata{"mode": setuidMode}))
|
||||
mode := statMode()
|
||||
assert.Equal(t, os.FileMode(0755), mode.Perm())
|
||||
assert.NotZero(t, mode&os.ModeSetuid,
|
||||
fmt.Sprintf("setuid bit should be restored with the flag, got %v", mode))
|
||||
})
|
||||
}
|
||||
|
||||
func TestFilter(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
r := fstest.NewRun(t)
|
||||
|
||||
@@ -140,15 +140,22 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) {
|
||||
if mode >= 0 {
|
||||
umode := uint(mode)
|
||||
if umode <= math.MaxUint32 {
|
||||
fileMode := os.FileMode(umode)
|
||||
// fileMode comes from the source, which may be untrusted, so
|
||||
// by default apply only the permission bits and strip the
|
||||
// setuid, setgid and sticky bits.
|
||||
if !o.fs.opt.MetadataRestoreSpecial {
|
||||
fileMode = fileMode.Perm()
|
||||
}
|
||||
if o.translatedLink {
|
||||
if haveLChmod {
|
||||
err = lChmod(o.path, os.FileMode(umode))
|
||||
err = lChmod(o.path, fileMode)
|
||||
} else {
|
||||
fs.Debugf(o, "Unable to set mode %v on a symlink on this OS", os.FileMode(umode))
|
||||
fs.Debugf(o, "Unable to set mode %v on a symlink on this OS", fileMode)
|
||||
err = nil
|
||||
}
|
||||
} else {
|
||||
err = os.Chmod(o.path, os.FileMode(umode))
|
||||
err = os.Chmod(o.path, fileMode)
|
||||
}
|
||||
if err != nil {
|
||||
outErr = fmt.Errorf("failed to change permissions: %w", err)
|
||||
|
||||
Reference in New Issue
Block a user