Make fsgofer attach more strict
Refuse to mount paths with "." and ".." in the path to prevent a compromised Sentry to mount "../../secrets". Only allow Attach to be called once per mount point. PiperOrigin-RevId: 199225929 Change-Id: I2a3eb7ea0b23f22eb8dde2e383e32563ec003bd5
This commit is contained in:
parent
6c585b8eb6
commit
19a0e83b50
|
@ -26,7 +26,6 @@ import (
|
|||
"math"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
|
@ -83,6 +82,9 @@ type Config struct {
|
|||
type attachPoint struct {
|
||||
prefix string
|
||||
conf Config
|
||||
|
||||
mu sync.Mutex
|
||||
attached bool
|
||||
}
|
||||
|
||||
// NewAttachPoint creates a new attacher that gives local file
|
||||
|
@ -93,19 +95,22 @@ func NewAttachPoint(prefix string, c Config) p9.Attacher {
|
|||
|
||||
// Attach implements p9.Attacher.
|
||||
func (a *attachPoint) Attach(appPath string) (p9.File, error) {
|
||||
// Only proceed if 'appPath' is valid.
|
||||
if !path.IsAbs(appPath) {
|
||||
return nil, fmt.Errorf("invalid path %q", appPath)
|
||||
}
|
||||
if path.Clean(appPath) != appPath {
|
||||
return nil, fmt.Errorf("invalid path %q", appPath)
|
||||
}
|
||||
|
||||
root := filepath.Join(a.prefix, appPath)
|
||||
root := path.Join(a.prefix, appPath)
|
||||
fi, err := os.Stat(root)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
mode := syscall.O_RDWR
|
||||
mode := os.O_RDWR
|
||||
if a.conf.ROMount || fi.IsDir() {
|
||||
mode = syscall.O_RDONLY
|
||||
mode = os.O_RDONLY
|
||||
}
|
||||
|
||||
f, err := os.OpenFile(root, mode|openFlags, 0)
|
||||
|
@ -114,8 +119,18 @@ func (a *attachPoint) Attach(appPath string) (p9.File, error) {
|
|||
}
|
||||
stat, err := stat(int(f.Fd()))
|
||||
if err != nil {
|
||||
f.Close()
|
||||
return nil, fmt.Errorf("failed to stat file %q, err: %v", root, err)
|
||||
}
|
||||
|
||||
a.mu.Lock()
|
||||
defer a.mu.Unlock()
|
||||
if a.attached {
|
||||
f.Close()
|
||||
return nil, fmt.Errorf("attach point already attached, prefix: %s", a.prefix)
|
||||
}
|
||||
a.attached = true
|
||||
|
||||
return newLocalFile(a.conf, f, root, stat)
|
||||
}
|
||||
|
||||
|
|
|
@ -19,6 +19,7 @@ import (
|
|||
"io/ioutil"
|
||||
"os"
|
||||
"path"
|
||||
"strings"
|
||||
"syscall"
|
||||
"testing"
|
||||
|
||||
|
@ -622,3 +623,45 @@ func TestAttachFile(t *testing.T) {
|
|||
t.Fatalf("ReadAt() wrong data, got: %s, expected: %s", string(rBuf), "foobar")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAttachError(t *testing.T) {
|
||||
conf := Config{ROMount: false}
|
||||
root, err := ioutil.TempDir("", "root-")
|
||||
if err != nil {
|
||||
t.Fatalf("ioutil.TempDir() failed, err: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(root)
|
||||
a := NewAttachPoint(root, conf)
|
||||
|
||||
c := path.Join(root, "test")
|
||||
if err := os.Mkdir(c, 0700); err != nil {
|
||||
t.Fatalf("os.Create(%q) failed, err: %v", c, err)
|
||||
}
|
||||
|
||||
for _, p := range []string{"test", "/test/../", "/test/./", "/test//"} {
|
||||
_, err := a.Attach(p)
|
||||
if err == nil {
|
||||
t.Fatalf("Attach(%q) should have failed", p)
|
||||
}
|
||||
if want := "invalid path"; !strings.Contains(err.Error(), want) {
|
||||
t.Fatalf("Attach(%q) wrong error, got: %v, wanted: %v", p, err, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestDoubleAttachError(t *testing.T) {
|
||||
conf := Config{ROMount: false}
|
||||
root, err := ioutil.TempDir("", "root-")
|
||||
if err != nil {
|
||||
t.Fatalf("ioutil.TempDir() failed, err: %v", err)
|
||||
}
|
||||
defer os.RemoveAll(root)
|
||||
a := NewAttachPoint(root, conf)
|
||||
|
||||
if _, err := a.Attach("/"); err != nil {
|
||||
t.Fatalf("Attach(%q) failed: %v", "/", err)
|
||||
}
|
||||
if _, err := a.Attach("/"); err == nil {
|
||||
t.Fatalf("Attach(%q) should have failed", "test")
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue