From e8fa3222311c25a16d770356c3741fe61cb24df5 Mon Sep 17 00:00:00 2001 From: Leona Maroni Date: Sat, 16 Nov 2024 22:03:14 +0100 Subject: [PATCH] rclone: apply patch for CVE-2024-52522 --- .../sync/rclone/CVE-2024-52522.patch | 441 ++++++++++++++++++ .../networking/sync/rclone/default.nix | 4 + 2 files changed, 445 insertions(+) create mode 100644 pkgs/applications/networking/sync/rclone/CVE-2024-52522.patch diff --git a/pkgs/applications/networking/sync/rclone/CVE-2024-52522.patch b/pkgs/applications/networking/sync/rclone/CVE-2024-52522.patch new file mode 100644 index 000000000000..77092cd7b022 --- /dev/null +++ b/pkgs/applications/networking/sync/rclone/CVE-2024-52522.patch @@ -0,0 +1,441 @@ +From a314da376573a48317515c95d7b529b63c219c65 Mon Sep 17 00:00:00 2001 +From: Leona Maroni +Date: Thu, 14 Nov 2024 16:13:57 +0000 +Subject: [PATCH] Patch for CVE-2024-52522 + +Before this change, if writing to a local backend with --metadata and +--links, if the incoming metadata contained mode or ownership +information then rclone would apply the mode/ownership to the +destination of the link not the link itself. + +This fixes the problem by using the link safe sycall variants +lchown/fchmodat when --links and --metadata is in use. Note that Linux +does not support setting permissions on symlinks, so rclone emits a +debug message in this case. + +This also fixes setting times on symlinks on Windows which wasn't +implemented for atime, mtime and was incorrectly setting the target of +the symlink for btime. + +See: https://github.com/rclone/rclone/security/advisories/GHSA-hrxh-9w67-g4cv +(cherry picked from commit 1e2b354456882ed22d8674b8bf37de55e0069514) + +Original author: Nick Craig-Wood +--- + backend/local/lchmod.go | 16 ++++++ + backend/local/lchmod_unix.go | 41 +++++++++++++++ + backend/local/lchtimes.go | 4 +- + backend/local/lchtimes_windows.go | 19 +++++++ + backend/local/local_internal_test.go | 76 ++++++++++++++++++++++++---- + backend/local/metadata.go | 33 ++++++++++-- + backend/local/setbtime.go | 6 +++ + backend/local/setbtime_windows.go | 37 ++++++++++++-- + 8 files changed, 211 insertions(+), 21 deletions(-) + create mode 100644 backend/local/lchmod.go + create mode 100644 backend/local/lchmod_unix.go + create mode 100644 backend/local/lchtimes_windows.go + +diff --git a/backend/local/lchmod.go b/backend/local/lchmod.go +new file mode 100644 +index 000000000..823718dfe +--- /dev/null ++++ b/backend/local/lchmod.go +@@ -0,0 +1,16 @@ ++//go:build windows || plan9 || js || linux ++ ++package local ++ ++import "os" ++ ++const haveLChmod = false ++ ++// lChmod changes the mode of the named file to mode. If the file is a symbolic ++// link, it changes the link, not the target. If there is an error, ++// it will be of type *PathError. ++func lChmod(name string, mode os.FileMode) error { ++ // Can't do this safely on this OS - chmoding a symlink always ++ // changes the destination. ++ return nil ++} +diff --git a/backend/local/lchmod_unix.go b/backend/local/lchmod_unix.go +new file mode 100644 +index 000000000..f1fdc4745 +--- /dev/null ++++ b/backend/local/lchmod_unix.go +@@ -0,0 +1,41 @@ ++//go:build !windows && !plan9 && !js && !linux ++ ++package local ++ ++import ( ++ "os" ++ "syscall" ++ ++ "golang.org/x/sys/unix" ++) ++ ++const haveLChmod = true ++ ++// syscallMode returns the syscall-specific mode bits from Go's portable mode bits. ++// ++// Borrowed from the syscall source since it isn't public. ++func syscallMode(i os.FileMode) (o uint32) { ++ o |= uint32(i.Perm()) ++ if i&os.ModeSetuid != 0 { ++ o |= syscall.S_ISUID ++ } ++ if i&os.ModeSetgid != 0 { ++ o |= syscall.S_ISGID ++ } ++ if i&os.ModeSticky != 0 { ++ o |= syscall.S_ISVTX ++ } ++ return o ++} ++ ++// lChmod changes the mode of the named file to mode. If the file is a symbolic ++// link, it changes the link, not the target. If there is an error, ++// it will be of type *PathError. ++func lChmod(name string, mode os.FileMode) error { ++ // NB linux does not support AT_SYMLINK_NOFOLLOW as a parameter to fchmodat ++ // and returns ENOTSUP if you try, so we don't support this on linux ++ if e := unix.Fchmodat(unix.AT_FDCWD, name, syscallMode(mode), unix.AT_SYMLINK_NOFOLLOW); e != nil { ++ return &os.PathError{Op: "lChmod", Path: name, Err: e} ++ } ++ return nil ++} +diff --git a/backend/local/lchtimes.go b/backend/local/lchtimes.go +index 97f3b1be1..38ec73dfe 100644 +--- a/backend/local/lchtimes.go ++++ b/backend/local/lchtimes.go +@@ -1,5 +1,5 @@ +-//go:build windows || plan9 || js +-// +build windows plan9 js ++//go:build plan9 || js ++// +build plan9 js + + package local + +diff --git a/backend/local/lchtimes_windows.go b/backend/local/lchtimes_windows.go +new file mode 100644 +index 000000000..a6dec9a12 +--- /dev/null ++++ b/backend/local/lchtimes_windows.go +@@ -0,0 +1,19 @@ ++//go:build windows ++ ++package local ++ ++import ( ++ "time" ++) ++ ++const haveLChtimes = true ++ ++// lChtimes changes the access and modification times of the named ++// link, similar to the Unix utime() or utimes() functions. ++// ++// The underlying filesystem may truncate or round the values to a ++// less precise time unit. ++// If there is an error, it will be of type *PathError. ++func lChtimes(name string, atime time.Time, mtime time.Time) error { ++ return setTimes(name, atime, mtime, time.Time{}, true) ++} +diff --git a/backend/local/local_internal_test.go b/backend/local/local_internal_test.go +index 06e47bed2..fc3fcce41 100644 +--- a/backend/local/local_internal_test.go ++++ b/backend/local/local_internal_test.go +@@ -251,22 +251,66 @@ func TestMetadata(t *testing.T) { + r := fstest.NewRun(t) + const filePath = "metafile.txt" + when := time.Now() +- const dayLength = len("2001-01-01") +- whenRFC := when.Format(time.RFC3339Nano) + r.WriteFile(filePath, "metadata file contents", when) + f := r.Flocal.(*Fs) + ++ // Set fs into "-l" / "--links" mode ++ f.opt.TranslateSymlinks = true ++ ++ // Write a symlink to the file ++ symlinkPath := "metafile-link.txt" ++ osSymlinkPath := filepath.Join(f.root, symlinkPath) ++ symlinkPath += linkSuffix ++ require.NoError(t, os.Symlink(filePath, osSymlinkPath)) ++ symlinkModTime := fstest.Time("2002-02-03T04:05:10.123123123Z") ++ require.NoError(t, lChtimes(osSymlinkPath, symlinkModTime, symlinkModTime)) ++ + // Get the object + obj, err := f.NewObject(ctx, filePath) + require.NoError(t, err) + o := obj.(*Object) + ++ // Get the symlink object ++ symlinkObj, err := f.NewObject(ctx, symlinkPath) ++ require.NoError(t, err) ++ symlinkO := symlinkObj.(*Object) ++ ++ // Record metadata for o ++ oMeta, err := o.Metadata(ctx) ++ require.NoError(t, err) ++ ++ // Test symlink first to check it doesn't mess up file ++ t.Run("Symlink", func(t *testing.T) { ++ testMetadata(t, r, symlinkO, symlinkModTime) ++ }) ++ ++ // Read it again ++ oMetaNew, err := o.Metadata(ctx) ++ require.NoError(t, err) ++ ++ // Check that operating on the symlink didn't change the file it was pointing to ++ // See: https://github.com/rclone/rclone/security/advisories/GHSA-hrxh-9w67-g4cv ++ assert.Equal(t, oMeta, oMetaNew, "metadata setting on symlink messed up file") ++ ++ // Now run the same tests on the file ++ t.Run("File", func(t *testing.T) { ++ testMetadata(t, r, o, when) ++ }) ++} ++ ++func testMetadata(t *testing.T, r *fstest.Run, o *Object, when time.Time) { ++ ctx := context.Background() ++ whenRFC := when.Format(time.RFC3339Nano) ++ const dayLength = len("2001-01-01") ++ ++ f := r.Flocal.(*Fs) + features := f.Features() + +- var hasXID, hasAtime, hasBtime bool ++ var hasXID, hasAtime, hasBtime, canSetXattrOnLinks bool + switch runtime.GOOS { + case "darwin", "freebsd", "netbsd", "linux": + hasXID, hasAtime, hasBtime = true, true, true ++ canSetXattrOnLinks = runtime.GOOS != "linux" + case "openbsd", "solaris": + hasXID, hasAtime = true, true + case "windows": +@@ -289,6 +333,10 @@ func TestMetadata(t *testing.T) { + require.NoError(t, err) + assert.Nil(t, m) + ++ if !canSetXattrOnLinks && o.translatedLink { ++ t.Skip("Skip remainder of test as can't set xattr on symlinks on this OS") ++ } ++ + inM := fs.Metadata{ + "potato": "chips", + "cabbage": "soup", +@@ -303,18 +351,21 @@ func TestMetadata(t *testing.T) { + }) + + checkTime := func(m fs.Metadata, key string, when time.Time) { ++ t.Helper() + mt, ok := o.parseMetadataTime(m, key) + assert.True(t, ok) + dt := mt.Sub(when) + precision := time.Second +- assert.True(t, dt >= -precision && dt <= precision, fmt.Sprintf("%s: dt %v outside +/- precision %v", key, dt, precision)) ++ assert.True(t, dt >= -precision && dt <= precision, fmt.Sprintf("%s: dt %v outside +/- precision %v want %v got %v", key, dt, precision, mt, when)) + } + + checkInt := func(m fs.Metadata, key string, base int) int { ++ t.Helper() + value, ok := o.parseMetadataInt(m, key, base) + assert.True(t, ok) + return value + } ++ + t.Run("Read", func(t *testing.T) { + m, err := o.Metadata(ctx) + require.NoError(t, err) +@@ -324,13 +375,12 @@ func TestMetadata(t *testing.T) { + checkInt(m, "mode", 8) + checkTime(m, "mtime", when) + +- assert.Equal(t, len(whenRFC), len(m["mtime"])) + assert.Equal(t, whenRFC[:dayLength], m["mtime"][:dayLength]) + +- if hasAtime { ++ if hasAtime && !o.translatedLink { // symlinks generally don't record atime + checkTime(m, "atime", when) + } +- if hasBtime { ++ if hasBtime && !o.translatedLink { // symlinks generally don't record btime + checkTime(m, "btime", when) + } + if hasXID { +@@ -354,6 +404,10 @@ func TestMetadata(t *testing.T) { + "mode": "0767", + "potato": "wedges", + } ++ if !canSetXattrOnLinks && o.translatedLink { ++ // Don't change xattr if not supported on symlinks ++ delete(newM, "potato") ++ } + err := o.writeMetadata(newM) + require.NoError(t, err) + +@@ -363,7 +417,11 @@ func TestMetadata(t *testing.T) { + + mode := checkInt(m, "mode", 8) + if runtime.GOOS != "windows" { +- assert.Equal(t, 0767, mode&0777, fmt.Sprintf("mode wrong - expecting 0767 got 0%o", mode&0777)) ++ expectedMode := 0767 ++ if o.translatedLink && runtime.GOOS == "linux" { ++ expectedMode = 0777 // perms of symlinks always read as 0777 on linux ++ } ++ assert.Equal(t, expectedMode, mode&0777, fmt.Sprintf("mode wrong - expecting 0%o got 0%o", expectedMode, mode&0777)) + } + + checkTime(m, "mtime", newMtime) +@@ -373,7 +431,7 @@ func TestMetadata(t *testing.T) { + if haveSetBTime { + checkTime(m, "btime", newBtime) + } +- if xattrSupported { ++ if xattrSupported && (canSetXattrOnLinks || !o.translatedLink) { + assert.Equal(t, "wedges", m["potato"]) + } + }) +diff --git a/backend/local/metadata.go b/backend/local/metadata.go +index 097abedf1..a4995714e 100644 +--- a/backend/local/metadata.go ++++ b/backend/local/metadata.go +@@ -2,6 +2,7 @@ package local + + import ( + "fmt" ++ "math" + "os" + "runtime" + "strconv" +@@ -104,7 +105,11 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) { + } + if haveSetBTime { + if btimeOK { +- err = setBTime(o.path, btime) ++ if o.translatedLink { ++ err = lsetBTime(o.path, btime) ++ } else { ++ err = setBTime(o.path, btime) ++ } + if err != nil { + outErr = fmt.Errorf("failed to set birth (creation) time: %w", err) + } +@@ -120,7 +125,11 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) { + if runtime.GOOS == "windows" || runtime.GOOS == "plan9" { + fs.Debugf(o, "Ignoring request to set ownership %o.%o on this OS", gid, uid) + } else { +- err = os.Chown(o.path, uid, gid) ++ if o.translatedLink { ++ err = os.Lchown(o.path, uid, gid) ++ } else { ++ err = os.Chown(o.path, uid, gid) ++ } + if err != nil { + outErr = fmt.Errorf("failed to change ownership: %w", err) + } +@@ -128,9 +137,23 @@ func (o *Object) writeMetadataToFile(m fs.Metadata) (outErr error) { + } + mode, hasMode := o.parseMetadataInt(m, "mode", 8) + if hasMode { +- err = os.Chmod(o.path, os.FileMode(mode)) +- if err != nil { +- outErr = fmt.Errorf("failed to change permissions: %w", err) ++ if mode >= 0 { ++ umode := uint(mode) ++ if umode <= math.MaxUint32 { ++ if o.translatedLink { ++ if haveLChmod { ++ err = lChmod(o.path, os.FileMode(umode)) ++ } else { ++ fs.Debugf(o, "Unable to set mode %v on a symlink on this OS", os.FileMode(umode)) ++ err = nil ++ } ++ } else { ++ err = os.Chmod(o.path, os.FileMode(umode)) ++ } ++ if err != nil { ++ outErr = fmt.Errorf("failed to change permissions: %w", err) ++ } ++ } + } + } + // FIXME not parsing rdev yet +diff --git a/backend/local/setbtime.go b/backend/local/setbtime.go +index 5d2c462ee..c85310916 100644 +--- a/backend/local/setbtime.go ++++ b/backend/local/setbtime.go +@@ -14,3 +14,9 @@ func setBTime(name string, btime time.Time) error { + // Does nothing + return nil + } ++ ++// lsetBTime changes the birth time of the link passed in ++func lsetBTime(name string, btime time.Time) error { ++ // Does nothing ++ return nil ++} +diff --git a/backend/local/setbtime_windows.go b/backend/local/setbtime_windows.go +index 2a46f09eb..b04a2cfb5 100644 +--- a/backend/local/setbtime_windows.go ++++ b/backend/local/setbtime_windows.go +@@ -10,15 +10,20 @@ import ( + + const haveSetBTime = true + +-// setBTime sets the birth time of the file passed in +-func setBTime(name string, btime time.Time) (err error) { ++// setTimes sets any of atime, mtime or btime ++// if link is set it sets a link rather than the target ++func setTimes(name string, atime, mtime, btime time.Time, link bool) (err error) { + pathp, err := syscall.UTF16PtrFromString(name) + if err != nil { + return err + } ++ fileFlag := uint32(syscall.FILE_FLAG_BACKUP_SEMANTICS) ++ if link { ++ fileFlag |= syscall.FILE_FLAG_OPEN_REPARSE_POINT ++ } + h, err := syscall.CreateFile(pathp, + syscall.FILE_WRITE_ATTRIBUTES, syscall.FILE_SHARE_WRITE, nil, +- syscall.OPEN_EXISTING, syscall.FILE_FLAG_BACKUP_SEMANTICS, 0) ++ syscall.OPEN_EXISTING, fileFlag, 0) + if err != nil { + return err + } +@@ -28,6 +33,28 @@ func setBTime(name string, btime time.Time) (err error) { + err = closeErr + } + }() +- bFileTime := syscall.NsecToFiletime(btime.UnixNano()) +- return syscall.SetFileTime(h, &bFileTime, nil, nil) ++ var patime, pmtime, pbtime *syscall.Filetime ++ if !atime.IsZero() { ++ t := syscall.NsecToFiletime(atime.UnixNano()) ++ patime = &t ++ } ++ if !mtime.IsZero() { ++ t := syscall.NsecToFiletime(mtime.UnixNano()) ++ pmtime = &t ++ } ++ if !btime.IsZero() { ++ t := syscall.NsecToFiletime(btime.UnixNano()) ++ pbtime = &t ++ } ++ return syscall.SetFileTime(h, pbtime, patime, pmtime) ++} ++ ++// setBTime sets the birth time of the file passed in ++func setBTime(name string, btime time.Time) (err error) { ++ return setTimes(name, time.Time{}, time.Time{}, btime, false) ++} ++ ++// lsetBTime changes the birth time of the link passed in ++func lsetBTime(name string, btime time.Time) error { ++ return setTimes(name, time.Time{}, time.Time{}, btime, true) + } +-- +2.47.0 + diff --git a/pkgs/applications/networking/sync/rclone/default.nix b/pkgs/applications/networking/sync/rclone/default.nix index 0ffd47435ff2..32f778c6aa9c 100644 --- a/pkgs/applications/networking/sync/rclone/default.nix +++ b/pkgs/applications/networking/sync/rclone/default.nix @@ -15,6 +15,10 @@ buildGoModule rec { hash = "sha256-75RnAROICtRUDn95gSCNO0F6wes4CkJteNfUN38GQIY="; }; + patches = [ + ./CVE-2024-52522.patch + ]; + vendorHash = "sha256-zGBwgIuabLDqWbutvPHDbPRo5Dd9kNfmgToZXy7KVgI="; subPackages = [ "." ];