]> Cypherpunks repositories - gostls13.git/commitdiff
os/exec: LookPath: use eaccess for exec check on linux
authorKir Kolyshkin <kolyshkin@gmail.com>
Tue, 28 Jun 2022 22:40:29 +0000 (15:40 -0700)
committerGopher Robot <gobot@golang.org>
Wed, 7 Sep 2022 01:09:45 +0000 (01:09 +0000)
Having an executable bit set for a binary is not enough for it to be
executable -- there might be more checks in the kernel. For example,
binaries on a filesystem mounted with "noexec" flag couldn't be
executed. There might be other scenarios involving ACLs, SELinux,
file capabilities, and so on.

As a result, LookPath might either find a non-executable (while going
over $PATH elements), or return a false positive that the argument
provided is an executable.

One possible fix would be to perform the check by using access(2)
syscall with X_OK flag.

Now, since access(2) uses real (rather than effective) uid and gid,
when used by a setuid or setgid binary, it checks permissions of the
(real) user who started the binary, rather than the actual effective
permissions. Therefore, using access with X_OK won't work as expected
for setuid/setgid binaries.

To fix this, modern platforms added ways to check against effective uid
and gid, with the most common being the faccessat(2) call with the
AT_EACCESS flag, as described by POSIX.1-2008 (in Linux, only
faccessat2(2) supports flags such as AT_EACCESS). Let's use it, and fall
back to checking permission bits if faccessat is not available.

Wrap the logic into unix.Eaccess, which is currently only implemented on
Linux. While many other OSes (Free/Net/OpenBSD, AIX, Solaris/Illumos, and
Darwin) do implement faccessat(2) with AT_EACCESS, it is not wired in
syscall package (except for AIX), so these platforms are left out for now.
In the future, eaccess can be implemented for these OSes, too.

Alas, a call to unix.Eaccess is not enough since we have to filter out
directories, so use both stat and Eaccess.

One minor change introduced by this commit is that LookPath and Command
now returns "is a directory" error when the argument contains a slash
and is a directory.  This is similar to what e.g. bash does on Linux:

$ bash -c /etc
bash: line 1: /etc: Is a directory

Add a test case, which, unfortunately, requires root, is specific to
Linux, and needs a relatively new kernel (supporting faccessat2).  Other
platforms either have different semantics for tmpfs with noexec, or have
different ways to set up a binary which has x bit set but nevertheless
could not be executed.

Change-Id: If49b6ef6bf4dd23b2c32bebec8832d83e511a4bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/414824
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
src/internal/syscall/unix/at_sysnum_linux.go
src/internal/syscall/unix/constants.go [new file with mode: 0644]
src/internal/syscall/unix/eaccess_linux.go [new file with mode: 0644]
src/internal/syscall/unix/eaccess_other.go [new file with mode: 0644]
src/os/exec/lp_linux_test.go [new file with mode: 0644]
src/os/exec/lp_unix.go

index fa7cd75d42653a7e41b4e91c8e37d72ac01f8108..b9b8495e32ac3ceb7a03c86378c13664f148e348 100644 (file)
@@ -9,5 +9,9 @@ import "syscall"
 const unlinkatTrap uintptr = syscall.SYS_UNLINKAT
 const openatTrap uintptr = syscall.SYS_OPENAT
 
-const AT_REMOVEDIR = 0x200
-const AT_SYMLINK_NOFOLLOW = 0x100
+const (
+       AT_EACCESS          = 0x200
+       AT_FDCWD            = -0x64
+       AT_REMOVEDIR        = 0x200
+       AT_SYMLINK_NOFOLLOW = 0x100
+)
diff --git a/src/internal/syscall/unix/constants.go b/src/internal/syscall/unix/constants.go
new file mode 100644 (file)
index 0000000..e324589
--- /dev/null
@@ -0,0 +1,13 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build unix
+
+package unix
+
+const (
+       R_OK = 0x4
+       W_OK = 0x2
+       X_OK = 0x1
+)
diff --git a/src/internal/syscall/unix/eaccess_linux.go b/src/internal/syscall/unix/eaccess_linux.go
new file mode 100644 (file)
index 0000000..5695a5e
--- /dev/null
@@ -0,0 +1,11 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package unix
+
+import "syscall"
+
+func Eaccess(path string, mode uint32) error {
+       return syscall.Faccessat(AT_FDCWD, path, mode, AT_EACCESS)
+}
diff --git a/src/internal/syscall/unix/eaccess_other.go b/src/internal/syscall/unix/eaccess_other.go
new file mode 100644 (file)
index 0000000..23be118
--- /dev/null
@@ -0,0 +1,13 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build unix && !linux
+
+package unix
+
+import "syscall"
+
+func Eaccess(path string, mode uint32) error {
+       return syscall.ENOSYS
+}
diff --git a/src/os/exec/lp_linux_test.go b/src/os/exec/lp_linux_test.go
new file mode 100644 (file)
index 0000000..96051b5
--- /dev/null
@@ -0,0 +1,69 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package exec
+
+import (
+       "internal/syscall/unix"
+       "os"
+       "path/filepath"
+       "syscall"
+       "testing"
+)
+
+func TestFindExecutableVsNoexec(t *testing.T) {
+       // This test case relies on faccessat2(2) syscall, which appeared in Linux v5.8.
+       if major, minor := unix.KernelVersion(); major < 5 || (major == 5 && minor < 8) {
+               t.Skip("requires Linux kernel v5.8 with faccessat2(2) syscall")
+       }
+
+       tmp := t.TempDir()
+
+       // Create a tmpfs mount.
+       err := syscall.Mount("tmpfs", tmp, "tmpfs", 0, "")
+       if err != nil {
+               if os.Geteuid() == 0 {
+                       t.Fatalf("tmpfs mount failed: %v", err)
+               }
+               // Requires root or CAP_SYS_ADMIN.
+               t.Skipf("requires ability to mount tmpfs (%v)", err)
+       }
+       t.Cleanup(func() {
+               if err := syscall.Unmount(tmp, 0); err != nil {
+                       t.Error(err)
+               }
+       })
+
+       // Create an executable.
+       path := filepath.Join(tmp, "program")
+       err = os.WriteFile(path, []byte("#!/bin/sh\necho 123\n"), 0o755)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       // Check that it works as expected.
+       err = findExecutable(path)
+       if err != nil {
+               t.Fatalf("findExecutable: got %v, want nil", err)
+       }
+
+       if err := Command(path).Run(); err != nil {
+               t.Fatalf("exec: got %v, want nil", err)
+       }
+
+       // Remount with noexec flag.
+       err = syscall.Mount("", tmp, "", syscall.MS_REMOUNT|syscall.MS_NOEXEC, "")
+       if err != nil {
+               t.Fatalf("remount %s with noexec failed: %v", tmp, err)
+       }
+
+       if err := Command(path).Run(); err == nil {
+               t.Fatal("exec on noexec filesystem: got nil, want error")
+       }
+
+       err = findExecutable(path)
+       if err == nil {
+               t.Fatalf("findExecutable: got nil, want error")
+       }
+}
index b2b412c96b6f9cbefd578975dfc465343003302a..af68c2f268a8422d7ae2c906ebab821c48081bda 100644 (file)
@@ -9,10 +9,12 @@ package exec
 import (
        "errors"
        "internal/godebug"
+       "internal/syscall/unix"
        "io/fs"
        "os"
        "path/filepath"
        "strings"
+       "syscall"
 )
 
 // ErrNotFound is the error resulting if a path search failed to find an executable file.
@@ -23,7 +25,18 @@ func findExecutable(file string) error {
        if err != nil {
                return err
        }
-       if m := d.Mode(); !m.IsDir() && m&0111 != 0 {
+       m := d.Mode()
+       if m.IsDir() {
+               return syscall.EISDIR
+       }
+       err = unix.Eaccess(file, unix.X_OK)
+       // ENOSYS means Eaccess is not available or not implemented.
+       // EPERM can be returned by Linux containers employing seccomp.
+       // In both cases, fall back to checking the permission bits.
+       if err == nil || (err != syscall.ENOSYS && err != syscall.EPERM) {
+               return err
+       }
+       if m&0111 != 0 {
                return nil
        }
        return fs.ErrPermission