From b402e8a6fc7685c6bf8061ff0917ee5735bc19c2 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 2 Nov 2024 17:44:55 +0100 Subject: [PATCH] fs: stricter enforcement to only call readdir on a directory Use O_DIRECTORY to prevent opening any other than a directory in readdirnames. --- internal/fs/const_unix.go | 3 +++ internal/fs/const_windows.go | 5 +++++ internal/fs/file.go | 5 +++-- internal/fs/file_unix_test.go | 22 ++++++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 internal/fs/file_unix_test.go diff --git a/internal/fs/const_unix.go b/internal/fs/const_unix.go index fe84cda17..e570c2553 100644 --- a/internal/fs/const_unix.go +++ b/internal/fs/const_unix.go @@ -7,3 +7,6 @@ import "syscall" // O_NOFOLLOW instructs the kernel to not follow symlinks when opening a file. const O_NOFOLLOW int = syscall.O_NOFOLLOW + +// O_DIRECTORY instructs the kernel to only open directories. +const O_DIRECTORY int = syscall.O_DIRECTORY diff --git a/internal/fs/const_windows.go b/internal/fs/const_windows.go index f1b263a54..4c29e0b9d 100644 --- a/internal/fs/const_windows.go +++ b/internal/fs/const_windows.go @@ -3,5 +3,10 @@ package fs +// TODO honor flags when opening files + // O_NOFOLLOW is a noop on Windows. const O_NOFOLLOW int = 0 + +// O_DIRECTORY is a noop on Windows. +const O_DIRECTORY int = 0 diff --git a/internal/fs/file.go b/internal/fs/file.go index 8d60ed159..c60625a07 100644 --- a/internal/fs/file.go +++ b/internal/fs/file.go @@ -64,9 +64,10 @@ func ResetPermissions(path string) error { return nil } -// Readdirnames returns a list of file in a directory. Flags are passed to fs.OpenFile. O_RDONLY is implied. +// Readdirnames returns a list of file in a directory. Flags are passed to fs.OpenFile. +// O_RDONLY and O_DIRECTORY are implied. func Readdirnames(filesystem FS, dir string, flags int) ([]string, error) { - f, err := filesystem.OpenFile(dir, O_RDONLY|flags, 0) + f, err := filesystem.OpenFile(dir, O_RDONLY|O_DIRECTORY|flags, 0) if err != nil { return nil, fmt.Errorf("openfile for readdirnames failed: %w", err) } diff --git a/internal/fs/file_unix_test.go b/internal/fs/file_unix_test.go new file mode 100644 index 000000000..00d68abb8 --- /dev/null +++ b/internal/fs/file_unix_test.go @@ -0,0 +1,22 @@ +//go:build unix + +package fs + +import ( + "path/filepath" + "syscall" + "testing" + + "github.com/restic/restic/internal/errors" + rtest "github.com/restic/restic/internal/test" +) + +func TestReaddirnamesFifo(t *testing.T) { + // should not block when reading from a fifo instead of a directory + tempdir := t.TempDir() + fifoFn := filepath.Join(tempdir, "fifo") + rtest.OK(t, mkfifo(fifoFn, 0o600)) + + _, err := Readdirnames(&Local{}, fifoFn, 0) + rtest.Assert(t, errors.Is(err, syscall.ENOTDIR), "unexpected error %v", err) +}