Merge pull request from GHSA-c72p-9xmj-rx3w

dmcgowan · web-flow · commit 7ad08c69e09e · 2021-07-19T10:30:51.000-07:00
[release/1.5] Use chmod path for checking symlink
diff --git a/archive/tar.go b/archive/tar.go
@@ -393,9 +393,8 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header
 		}
 	}
 
-	// There is no LChmod, so ignore mode for symlink. Also, this
-	// must happen after chown, as that can modify the file mode
-	if err := handleLChmod(hdr, path, hdrInfo); err != nil {
+	// call lchmod after lchown since lchown can modify the file mode
+	if err := lchmod(path, hdrInfo.Mode()); err != nil {
 		return err
 	}
 
diff --git a/archive/tar_freebsd.go b/archive/tar_freebsd.go
@@ -18,7 +18,11 @@
 
 package archive
 
-import "golang.org/x/sys/unix"
+import (
+	"os"
+
+	"golang.org/x/sys/unix"
+)
 
 // mknod wraps unix.Mknod.  FreeBSD's unix.Mknod signature is different from
 // other Unix and Unix-like operating systems.
@@ -34,3 +38,11 @@ func lsetxattrCreate(link string, attr string, data []byte) error {
 	}
 	return err
 }
+
+func lchmod(path string, mode os.FileMode) error {
+	err := unix.Fchmodat(unix.AT_FDCWD, path, uint32(mode), unix.AT_SYMLINK_NOFOLLOW)
+	if err != nil {
+		err = &os.PathError{Op: "lchmod", Path: path, Err: err}
+	}
+	return err
+}
diff --git a/archive/tar_mostunix.go b/archive/tar_mostunix.go
@@ -18,7 +18,11 @@
 
 package archive
 
-import "golang.org/x/sys/unix"
+import (
+	"os"
+
+	"golang.org/x/sys/unix"
+)
 
 // mknod wraps Unix.Mknod and casts dev to int
 func mknod(path string, mode uint32, dev uint64) error {
@@ -34,3 +38,18 @@ func lsetxattrCreate(link string, attr string, data []byte) error {
 	}
 	return err
 }
+
+// lchmod checks for symlink and changes the mode if not a symlink
+func lchmod(path string, mode os.FileMode) error {
+	fi, err := os.Lstat(path)
+	if err != nil {
+		return err
+	}
+
+	if fi.Mode()&os.ModeSymlink == 0 {
+		if err := os.Chmod(path, mode); err != nil {
+			return err
+		}
+	}
+	return nil
+}
diff --git a/archive/tar_test.go b/archive/tar_test.go
@@ -243,6 +243,11 @@ func TestBreakouts(t *testing.T) {
 		return nil
 	}
 	errFileDiff := errors.New("files differ")
+	td, err := ioutil.TempDir("", "test-breakouts-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(td)
 
 	isSymlinkFile := func(f string) func(string) error {
 		return func(root string) error {
@@ -744,6 +749,36 @@ func TestBreakouts(t *testing.T) {
 			// resolution ends up just removing etc
 			validator: fileNotExists("etc/passwd"),
 		},
+		{
+
+			name: "HardlinkSymlinkChmod",
+			w: func() tartest.WriterToTar {
+				p := filepath.Join(td, "perm400")
+				if err := ioutil.WriteFile(p, []byte("..."), 0400); err != nil {
+					t.Fatal(err)
+				}
+				ep := filepath.Join(td, "also-exists-outside-root")
+				if err := ioutil.WriteFile(ep, []byte("..."), 0640); err != nil {
+					t.Fatal(err)
+				}
+
+				return tartest.TarAll(
+					tc.Symlink(p, ep),
+					tc.Link(ep, "sketchylink"),
+				)
+			}(),
+			validator: func(string) error {
+				p := filepath.Join(td, "perm400")
+				fi, err := os.Lstat(p)
+				if err != nil {
+					return err
+				}
+				if perm := fi.Mode() & os.ModePerm; perm != 0400 {
+					return errors.Errorf("%s perm changed from 0400 to %04o", p, perm)
+				}
+				return nil
+			},
+		},
 	}
 
 	for _, bo := range breakouts {
diff --git a/archive/tar_unix.go b/archive/tar_unix.go
@@ -111,21 +111,6 @@ func handleTarTypeBlockCharFifo(hdr *tar.Header, path string) error {
 	return mknod(path, mode, unix.Mkdev(uint32(hdr.Devmajor), uint32(hdr.Devminor)))
 }
 
-func handleLChmod(hdr *tar.Header, path string, hdrInfo os.FileInfo) error {
-	if hdr.Typeflag == tar.TypeLink {
-		if fi, err := os.Lstat(hdr.Linkname); err == nil && (fi.Mode()&os.ModeSymlink == 0) {
-			if err := os.Chmod(path, hdrInfo.Mode()); err != nil && !os.IsNotExist(err) {
-				return err
-			}
-		}
-	} else if hdr.Typeflag != tar.TypeSymlink {
-		if err := os.Chmod(path, hdrInfo.Mode()); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func getxattr(path, attr string) ([]byte, error) {
 	b, err := sysx.LGetxattr(path, attr)
 	if err == unix.ENOTSUP || err == sysx.ENODATA {
diff --git a/archive/tar_windows.go b/archive/tar_windows.go
@@ -98,7 +98,7 @@ func handleTarTypeBlockCharFifo(hdr *tar.Header, path string) error {
 	return nil
 }
 
-func handleLChmod(hdr *tar.Header, path string, hdrInfo os.FileInfo) error {
+func lchmod(path string, mode os.FileMode) error {
 	return nil
 }
 

Original file line number	Diff line number	Diff line change
`@@ -393,9 +393,8 @@ func createTarFile(ctx context.Context, path, extractDir string, hdr *tar.Header`
`393`	`393`	`}`
`394`	`394`	`}`
`395`	`395`
`396`		`- // There is no LChmod, so ignore mode for symlink. Also, this`
`397`		`- // must happen after chown, as that can modify the file mode`
`398`		`- if err := handleLChmod(hdr, path, hdrInfo); err != nil {`
	`396`	`+ // call lchmod after lchown since lchown can modify the file mode`
	`397`	`+ if err := lchmod(path, hdrInfo.Mode()); err != nil {`
`399`	`398`	`return err`
`400`	`399`	`}`
`401`	`400`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ func handleTarTypeBlockCharFifo(hdr *tar.Header, path string) error {`
`98`	`98`	`return nil`
`99`	`99`	`}`
`100`	`100`
`101`		`-func handleLChmod(hdr *tar.Header, path string, hdrInfo os.FileInfo) error {`
	`101`	`+func lchmod(path string, mode os.FileMode) error {`
`102`	`102`	`return nil`
`103`	`103`	`}`
`104`	`104`