fix: reject system binaries unreachable by other users; consistent info

The system-install validation checked the binary's own permissions and
the parent's tamper-safety but never reachability: a 0755 root-owned
binary under a 0700 directory (e.g. /root/pmg) passed every check while
every non-root user's shim failed with exit 127. Walk the directory
chain to / and require the search bit for others; doctor's system
binary check inherits this. E2E gains a reject case for a binary under
a non-searchable directory.

setup info: render alias/user-shim/system-shim rows through one
installed-state formatter (location when installed, "not installed"
otherwise) instead of a mix of booleans, paths, and prose.
This commit is contained in:
Sahilb315
2026-07-14 14:06:17 +05:30
parent 748d40c14f
commit f251a073e3
5 changed files with 79 additions and 7 deletions
+13
View File
@@ -927,6 +927,19 @@ jobs:
fi
echo "SUCCESS: user-owned binary rejected"
- name: Reject unreachable PMG binary for system install
run: |
# Binary is 0755 and root-owned, but sits under a 0700 dir: other
# users cannot traverse to it, so every shim would exit 127.
sudo mkdir -p /root/pmg-unreachable
sudo install -m 755 bin/pmg /root/pmg-unreachable/pmg
sudo chmod 700 /root/pmg-unreachable
if sudo /root/pmg-unreachable/pmg setup install --system; then
echo "ERROR: system install accepted a binary under a non-searchable directory"
exit 1
fi
echo "SUCCESS: unreachable binary rejected"
- name: Install PMG system-wide
run: |
# GitHub runners ship /usr/local/bin world-writable; system install
+18 -6
View File
@@ -78,13 +78,13 @@ func executeSetupInfo() error {
}
shellEntries["Detected Shell"] = shell
shellEntries["Alias Installed"] = strconv.FormatBool(isInstalled)
shellEntries["User Shims"] = strconv.FormatBool(shim.UserShimsInstalled())
if shim.SystemShimsInstalled() {
shellEntries["System Shims"] = shim.SystemBinDir()
} else {
shellEntries["System Shims"] = "not installed"
shellEntries["Aliases"] = installedState(isInstalled, aliasManager.GetRcPath())
userBinDir, err := shim.UserBinDir()
if err != nil {
userBinDir = ""
}
shellEntries["User Shims"] = installedState(shim.UserShimsInstalled(), userBinDir)
shellEntries["System Shims"] = installedState(shim.SystemShimsInstalled(), shim.SystemBinDir())
ui.PrintInfoSection("Shell Integration", shellEntries)
// Security section
@@ -188,6 +188,18 @@ func executeSetupInfo() error {
return nil
}
// installedState renders installation-state rows consistently: the location
// when installed, "not installed" otherwise.
func installedState(installed bool, location string) string {
if !installed {
return "not installed"
}
if location == "" {
return "installed"
}
return fmt.Sprintf("installed (%s)", location)
}
func resolveSandboxDriverName() string {
sb, err := platform.NewSandbox()
if err != nil {
+1 -1
View File
@@ -8,7 +8,7 @@ sudo pmg setup install --system
**Requires Linux and root.** Install PMG as root into a standard system path such as `/usr/local/bin`. A user-local build (e.g. `~/go/bin/pmg`) is rejected.
`--system` enforces this because every user's shims run the PMG binary by absolute path. Before installing, it checks that the binary is **root-owned**, world-executable, not group- or other-writable, and located in a **root-owned directory** that isn't world-writable.
`--system` enforces this because every user's shims run the PMG binary by absolute path. Before installing, it checks that the binary is **root-owned**, world-executable, not group- or other-writable, located in a **root-owned directory** that isn't world-writable, and reachable through world-searchable directories (a binary under `/root`, mode 0700, is rejected because other users could never execute it).
Per-user `pmg setup install` remains available and does not conflict with a system install.
+25
View File
@@ -115,10 +115,33 @@ func validateSystemExecutable(path string) error {
if err := requireSafeParentDir(filepath.Dir(path)); err != nil {
return err
}
if err := requirePathSearchableByAll(path); err != nil {
return err
}
}
return nil
}
// requirePathSearchableByAll walks every directory from the binary's parent up
// to the filesystem root and requires the execute (search) bit for others. The
// shims exec the binary as arbitrary users, so a single non-searchable
// ancestor (e.g. /root, mode 0700) makes the path unreachable and every shim
// fail with exit 127 for non-root users, even when the binary itself is 0755.
func requirePathSearchableByAll(path string) error {
for dir := filepath.Dir(path); ; dir = filepath.Dir(dir) {
info, err := os.Stat(dir)
if err != nil {
return fmt.Errorf("failed to inspect directory %s: %w", dir, err)
}
if info.Mode().Perm()&0o001 == 0 {
return fmt.Errorf("directory %s is not searchable by all users, so pmg at %s would be unreachable from other accounts", dir, path)
}
if dir == filepath.Dir(dir) {
return nil
}
}
}
func requireRootOwnedPath(path string, info os.FileInfo) error {
uid, ok := fileOwnerUID(info)
if !ok {
@@ -134,6 +157,8 @@ func requireRootOwnedPath(path string, info os.FileInfo) error {
// not the full chain up to /. It requires a root-owned, non-world-writable
// parent so an unprivileged account cannot swap the shared binary that every
// user's shims exec; a maliciously writable grandparent is out of scope.
// (Reachability of the full chain is separately enforced by
// requirePathSearchableByAll.)
//
// Group-writable is allowed deliberately: Debian/Ubuntu ship /usr/local/bin as
// root:staff mode 2775, so rejecting group-writable would refuse the documented
+22
View File
@@ -204,6 +204,28 @@ func TestParseShimPMGBinRoundTripsShellQuote(t *testing.T) {
}
}
func TestRequirePathSearchableByAll(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("unix permission semantics")
}
t.Run("standard system path passes", func(t *testing.T) {
// Only directories are inspected, so the file itself need not exist.
assert.NoError(t, requirePathSearchableByAll("/usr/bin/pmg-does-not-exist"))
})
t.Run("non-searchable ancestor rejects", func(t *testing.T) {
base := t.TempDir()
require.NoError(t, os.Chmod(base, 0o700))
sub := filepath.Join(base, "sub")
require.NoError(t, os.MkdirAll(sub, 0o755))
err := requirePathSearchableByAll(filepath.Join(sub, "pmg"))
require.Error(t, err)
assert.Contains(t, err.Error(), "not searchable by all users")
})
}
func TestNewSystemShimManagerForRemoveSkipsValidation(t *testing.T) {
root := t.TempDir()
useSystemPaths(t, root)