From a2c14f8771b55f6b33d525eae2644c48c29823fa Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 23 Jul 2024 20:59:35 +1000 Subject: [PATCH] CHANGELOG: add readlinkat(fd, "") shout-out Signed-off-by: Aleksa Sarai --- CHANGELOG.md | 12 ++++++++++++ lookup_linux.go | 2 +- procfs_linux.go | 2 +- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9776849..b6e95ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/). operations for `openat2`, and a modest improvement for non-`openat2`) and is far more guaranteed to match the correct `openat2(RESOLVE_IN_ROOT)` behaviour. +- We now use `readlinkat(fd, "")` where possible. For `Open(at)InRoot` this + effectively just means that we no longer risk getting spurious errors during + rename races. However, for our hardened procfs handler, this in theory should + prevent mount attacks from tricking us when doing magic-link readlinks (even + when using the unsafe host `/proc` handle). Unfortunately `Reopen` is still + potentially vulnerable to those kinds of somewhat-esoteric attacks. + + Technically this [will only work on post-2.6.39 kernels][linux-readlinkat-emptypath] + but it seems incredibly unlikely anyone is using `filepath-securejoin` on a + pre-2011 kernel. ### Fixed ### - Several improvements were made to the errors returned by `Open(at)InRoot` and @@ -25,6 +35,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/). These changes were done to match `openat2`'s behaviour and purely is a consistency fix (most users are going to be using `openat2` anyway). +[linux-readlinkat-emptypath]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=65cfc6722361570bfe255698d9cd4dccaf47570d + ## [0.3.0] - 2024-07-11 ## ### Added ### diff --git a/lookup_linux.go b/lookup_linux.go index 103a743..290befa 100644 --- a/lookup_linux.go +++ b/lookup_linux.go @@ -283,7 +283,7 @@ func lookupInRoot(root *os.File, unsafePath string, partial bool) (Handle *os.Fi switch st.Mode() & os.ModeType { case os.ModeSymlink: - // readlinkat implies AT_EMPTY_PATH since Linux 2.6.38. See + // readlinkat implies AT_EMPTY_PATH since Linux 2.6.39. See // Linux commit 65cfc6722361 ("readlinkat(), fchownat() and // fstatat() with empty relative pathnames"). linkDest, err := readlinkatFile(nextDir, "") diff --git a/procfs_linux.go b/procfs_linux.go index fdb13d0..adf0bd0 100644 --- a/procfs_linux.go +++ b/procfs_linux.go @@ -401,7 +401,7 @@ func doRawProcSelfFdReadlink(procRoot *os.File, fd int) (string, error) { return "", fmt.Errorf("check safety of /proc/thread-self/fd/%d magiclink: %w", fd, err) } - // readlinkat implies AT_EMPTY_PATH since Linux 2.6.38. See Linux commit + // readlinkat implies AT_EMPTY_PATH since Linux 2.6.39. See Linux commit // 65cfc6722361 ("readlinkat(), fchownat() and fstatat() with empty // relative pathnames"). return readlinkatFile(procFdLink, "")