Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/sys/unix: Faccessat() fails to account for secondary group memberships when flags != 0 #39660

Closed
makholm opened this issue Jun 17, 2020 · 3 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Linux
Milestone

Comments

@makholm
Copy link

makholm commented Jun 17, 2020

Go's Facessat() implementation fails to account for secondary group memberships when flags != 0 when determining whether a given file access is allowed. This causes it to return an error where file access would actually be allowed. The following example demonstrates the problem, with a file that is only readable because of secondary group membership:

$ go version
go version go1.13.5 linux/amd64
$ cat test.go
package main

import (
	"fmt"
	"os"
	"syscall"
	"golang.org/x/sys/unix"
)

func main() {
	path := os.Args[1]
	err := syscall.Faccessat(0, path, unix.R_OK, unix.AT_EACCESS)
	fmt.Printf("uid %v, euid %v, gid %v, egid %v, path %v, err: %v\n", syscall.Getuid(), syscall.Geteuid(), syscall.Getgid(), syscall.Getegid(), path, err)
}
$ go build -o test
$ id
uid=501(lma) gid=20(dialout) groups=20(dialout),4(adm),24(cdrom),27(sudo),30(dip),46(plugdev),114(lpadmin),115(sambashare),116(docker)
$ ls -l file
-rwxrwx--- 1 root adm 6 Jun 17 20:19 file
$ cat file
Hello
$ ./test $(pwd)/file
uid 501, euid 501, gid 20, egid 20, path /home/lma/faccessat/file, err: permission denied
$

The go implementation (https://golang.org/src/syscall/syscall_linux.go) references the glibc implementation (https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/faccessat.c;hb=HEAD) but where glibc calls __group_member() to check group memberships if file's gid != user's gid, the go implementation just does a last-resort check of world/other permissions.

Naturally, I'd expect syscall.Faccessat() to act like C faccessat()... :)

@gopherbot gopherbot added this to the Unreleased milestone Jun 17, 2020
@ianlancetaylor ianlancetaylor added help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 18, 2020
@gopherbot
Copy link

Change https://golang.org/cl/238722 mentions this issue: unix: check secondary group membership for Faccessat(..., AT_EACCESS) on Linux

@tklauser
Copy link
Member

@makholm sent https://golang.org/cl/238722 with a possible fix (for x/sys/unix). It fixes the issue for me with the given example.

@gopherbot
Copy link

Change https://golang.org/cl/238937 mentions this issue: syscall: check secondary group membership for Faccessat(..., AT_EACCESS) on Linux

gopherbot pushed a commit that referenced this issue Jun 20, 2020
…SS) on Linux

Follow glibc's implementation and check secondary group memberships
using Getgroups.

No test since we cannot easily change file permissions when not running
as root and the test is meaningless if running as root.

Same as CL 238722 did for x/sys/unix

Updates #39660

Change-Id: I6af50e27b255e33405558947a0ab3dfbc33b2d50
Reviewed-on: https://go-review.googlesource.com/c/go/+/238937
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jun 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Linux
Projects
None yet
Development

No branches or pull requests

4 participants