diff --git a/backend/local/local.go b/backend/local/local.go index 2133279d0..91ec9cdce 100644 --- a/backend/local/local.go +++ b/backend/local/local.go @@ -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. @@ -347,24 +374,25 @@ only useful for reading. // Options defines the configuration for this backend type Options struct { - FollowSymlinks bool `config:"copy_links"` - TranslateSymlinks bool `config:"links"` - SkipSymlinks bool `config:"skip_links"` - SkipSpecials bool `config:"skip_specials"` - UTFNorm bool `config:"unicode_normalization"` - NoCheckUpdated bool `config:"no_check_updated"` - NoUNC bool `config:"nounc"` - OneFileSystem bool `config:"one_file_system"` - CaseSensitive bool `config:"case_sensitive"` - CaseInsensitive bool `config:"case_insensitive"` - NoPreAllocate bool `config:"no_preallocate"` - NoSparse bool `config:"no_sparse"` - NoSetModTime bool `config:"no_set_modtime"` - FatalIfNoSpace bool `config:"fatal_if_no_space"` - TimeType timeType `config:"time_type"` - Hashes fs.CommaSepList `config:"hashes"` - Enc encoder.MultiEncoder `config:"encoding"` - NoClone bool `config:"no_clone"` + FollowSymlinks bool `config:"copy_links"` + TranslateSymlinks bool `config:"links"` + SkipSymlinks bool `config:"skip_links"` + SkipSpecials bool `config:"skip_specials"` + UTFNorm bool `config:"unicode_normalization"` + NoCheckUpdated bool `config:"no_check_updated"` + NoUNC bool `config:"nounc"` + OneFileSystem bool `config:"one_file_system"` + CaseSensitive bool `config:"case_sensitive"` + CaseInsensitive bool `config:"case_insensitive"` + NoPreAllocate bool `config:"no_preallocate"` + NoSparse bool `config:"no_sparse"` + NoSetModTime bool `config:"no_set_modtime"` + FatalIfNoSpace bool `config:"fatal_if_no_space"` + TimeType timeType `config:"time_type"` + 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 diff --git a/backend/local/local_internal_test.go b/backend/local/local_internal_test.go index 2fd4fcaac..f36cc1791 100644 --- a/backend/local/local_internal_test.go +++ b/backend/local/local_internal_test.go @@ -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) diff --git a/backend/local/metadata.go b/backend/local/metadata.go index 75b195e64..1e04b5cfb 100644 --- a/backend/local/metadata.go +++ b/backend/local/metadata.go @@ -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)