From a5cbeeafea552709d27fe8e6b1ebcfcdde4d2afa Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Thu, 11 Jun 2026 18:50:11 +0200 Subject: [PATCH] fix(fs, model): improve symlink resilience in file shortcut (#10739) Ensure file was a file before the shortcut as well as after... (This was implied when talking to a correct implementation, but not enforced.) Make our file opening operations safe by default by ensuring the last path component is not a symlink. --------- Signed-off-by: Jakob Borg --- lib/fs/basicfs.go | 21 +++------------------ lib/fs/basicfs_unix.go | 3 +++ lib/fs/basicfs_windows.go | 2 ++ lib/model/folder_sendrecv.go | 2 +- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/lib/fs/basicfs.go b/lib/fs/basicfs.go index cf40f6fc8..9121fec20 100644 --- a/lib/fs/basicfs.go +++ b/lib/fs/basicfs.go @@ -241,15 +241,7 @@ func (f *BasicFilesystem) DirNames(name string) ([]string, error) { } func (f *BasicFilesystem) Open(name string) (File, error) { - rootedName, err := f.rooted(name) - if err != nil { - return nil, err - } - fd, err := os.Open(rootedName) - if err != nil { - return nil, err - } - return basicFile{fd, name}, err + return f.OpenFile(name, os.O_RDONLY, 0) } func (f *BasicFilesystem) OpenFile(name string, flags int, mode FileMode) (File, error) { @@ -257,6 +249,7 @@ func (f *BasicFilesystem) OpenFile(name string, flags int, mode FileMode) (File, if err != nil { return nil, err } + flags |= alwaysOpenFlags // enforce extra bits in flags fd, err := os.OpenFile(rootedName, flags, os.FileMode(mode)) if err != nil { return nil, err @@ -265,15 +258,7 @@ func (f *BasicFilesystem) OpenFile(name string, flags int, mode FileMode) (File, } func (f *BasicFilesystem) Create(name string) (File, error) { - rootedName, err := f.rooted(name) - if err != nil { - return nil, err - } - fd, err := os.Create(rootedName) - if err != nil { - return nil, err - } - return basicFile{fd, name}, err + return f.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0o666) } func (*BasicFilesystem) Walk(_ string, _ WalkFunc) error { diff --git a/lib/fs/basicfs_unix.go b/lib/fs/basicfs_unix.go index dc0f963ce..f9527f1bd 100644 --- a/lib/fs/basicfs_unix.go +++ b/lib/fs/basicfs_unix.go @@ -14,8 +14,11 @@ import ( "path/filepath" "strconv" "strings" + "syscall" ) +const alwaysOpenFlags = syscall.O_NOFOLLOW // never open symlinks as the final path component + func (f *BasicFilesystem) CreateSymlink(target, name string) error { name, err := f.rooted(name) if err != nil { diff --git a/lib/fs/basicfs_windows.go b/lib/fs/basicfs_windows.go index 34c3bb860..c6be9bae0 100644 --- a/lib/fs/basicfs_windows.go +++ b/lib/fs/basicfs_windows.go @@ -21,6 +21,8 @@ import ( "golang.org/x/sys/windows" ) +const alwaysOpenFlags = 0 // no extra flags + var errNotSupported = errors.New("symlinks not supported") func (BasicFilesystem) ReadSymlink(path string) (string, error) { diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index b9ea673c4..c04eb8b56 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -382,7 +382,7 @@ loop: if err != nil { return nil, nil, err } - if hasCurFile && file.BlocksEqual(curFile) { + if hasCurFile && curFile.Type == file.Type && file.BlocksEqual(curFile) { // We are supposed to copy the entire file, and then fetch nothing. We // are only updating metadata, so we don't actually *need* to make the // copy.