Fix creation of subpath with SUID/SGID directories.

SafeMakeDir() should apply SUID/SGID/sticky bits to the directory it creates.
pull/6/head
Jan Safranek 2018-03-16 16:58:47 +01:00
parent ca06cc43f7
commit e55164c42d
2 changed files with 65 additions and 5 deletions

View File

@ -1032,7 +1032,19 @@ func doSafeMakeDir(pathname string, base string, perm os.FileMode) error {
// so user can read/write it. This is the behavior of previous code. // so user can read/write it. This is the behavior of previous code.
// TODO: chmod all created directories, not just the last one. // TODO: chmod all created directories, not just the last one.
// parentFD is the last created directory. // parentFD is the last created directory.
if err = syscall.Fchmod(parentFD, uint32(perm)&uint32(os.ModePerm)); err != nil {
// Translate perm (os.FileMode) to uint32 that fchmod() expects
kernelPerm := uint32(perm & os.ModePerm)
if perm&os.ModeSetgid > 0 {
kernelPerm |= syscall.S_ISGID
}
if perm&os.ModeSetuid > 0 {
kernelPerm |= syscall.S_ISUID
}
if perm&os.ModeSticky > 0 {
kernelPerm |= syscall.S_ISVTX
}
if err = syscall.Fchmod(parentFD, kernelPerm); err != nil {
return fmt.Errorf("chmod %q failed: %s", currentPath, err) return fmt.Errorf("chmod %q failed: %s", currentPath, err)
} }
return nil return nil

View File

@ -418,7 +418,7 @@ func TestPathWithinBase(t *testing.T) {
} }
func TestSafeMakeDir(t *testing.T) { func TestSafeMakeDir(t *testing.T) {
defaultPerm := os.FileMode(0750) defaultPerm := os.FileMode(0750) + os.ModeDir
tests := []struct { tests := []struct {
name string name string
// Function that prepares directory structure for the test under given // Function that prepares directory structure for the test under given
@ -426,6 +426,7 @@ func TestSafeMakeDir(t *testing.T) {
prepare func(base string) error prepare func(base string) error
path string path string
checkPath string checkPath string
perm os.FileMode
expectError bool expectError bool
}{ }{
{ {
@ -435,6 +436,37 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"test/directory", "test/directory",
defaultPerm,
false,
},
{
"directory-with-sgid",
func(base string) error {
return nil
},
"test/directory",
"test/directory",
os.FileMode(0777) + os.ModeDir + os.ModeSetgid,
false,
},
{
"directory-with-suid",
func(base string) error {
return nil
},
"test/directory",
"test/directory",
os.FileMode(0777) + os.ModeDir + os.ModeSetuid,
false,
},
{
"directory-with-sticky-bit",
func(base string) error {
return nil
},
"test/directory",
"test/directory",
os.FileMode(0777) + os.ModeDir + os.ModeSticky,
false, false,
}, },
{ {
@ -444,6 +476,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"test/directory", "test/directory",
defaultPerm,
false, false,
}, },
{ {
@ -453,6 +486,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"", "",
"", "",
defaultPerm,
false, false,
}, },
{ {
@ -462,6 +496,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"..", "..",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -471,6 +506,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/../../..", "test/../../..",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -483,6 +519,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"destination/directory", "destination/directory",
defaultPerm,
false, false,
}, },
{ {
@ -492,6 +529,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -514,6 +552,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test1/dir/dir/dir/dir/dir/dir/dir/foo", "test1/dir/dir/dir/dir/dir/dir/dir/foo",
"test2/foo", "test2/foo",
defaultPerm,
false, false,
}, },
{ {
@ -523,6 +562,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -532,6 +572,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -541,6 +582,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test", "test",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -556,6 +598,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"dir/test", "dir/test",
"", "",
defaultPerm,
false, false,
}, },
{ {
@ -568,6 +611,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"dir/test", "dir/test",
"", "",
defaultPerm,
true, true,
}, },
{ {
@ -577,6 +621,7 @@ func TestSafeMakeDir(t *testing.T) {
}, },
"test/directory", "test/directory",
"", "",
defaultPerm,
true, true,
}, },
} }
@ -589,7 +634,7 @@ func TestSafeMakeDir(t *testing.T) {
} }
test.prepare(base) test.prepare(base)
pathToCreate := filepath.Join(base, test.path) pathToCreate := filepath.Join(base, test.path)
err = doSafeMakeDir(pathToCreate, base, defaultPerm) err = doSafeMakeDir(pathToCreate, base, test.perm)
if err != nil && !test.expectError { if err != nil && !test.expectError {
t.Errorf("test %q: %s", test.name, err) t.Errorf("test %q: %s", test.name, err)
} }
@ -601,14 +646,17 @@ func TestSafeMakeDir(t *testing.T) {
} }
if test.checkPath != "" { if test.checkPath != "" {
if _, err := os.Stat(filepath.Join(base, test.checkPath)); err != nil { st, err := os.Stat(filepath.Join(base, test.checkPath))
if err != nil {
t.Errorf("test %q: cannot read path %s", test.name, test.checkPath) t.Errorf("test %q: cannot read path %s", test.name, test.checkPath)
} }
if st.Mode() != test.perm {
t.Errorf("test %q: expected permissions %o, got %o", test.name, test.perm, st.Mode())
}
} }
os.RemoveAll(base) os.RemoveAll(base)
} }
} }
func validateDirEmpty(dir string) error { func validateDirEmpty(dir string) error {