moby · thaJeztah · Feb 24, 2022 · Jan 24, 2022 · Feb 18, 2022
diff --git a/Dockerfile b/Dockerfile
@@ -2,7 +2,6 @@
 
 ARG CROSS="false"
 ARG SYSTEMD="false"
-# IMPORTANT: When updating this please note that stdlib archive/tar pkg is vendored
 ARG GO_VERSION=1.17.7
 ARG DEBIAN_FRONTEND=noninteractive
 ARG VPNKIT_VERSION=0.5.0

@@ -10,19 +10,5 @@ set -x
 SCRIPTDIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 "${SCRIPTDIR}"/go-mod-prepare.sh
 
-if [ $# -eq 0 ] || [ "$1" != "archive/tar" ]; then
-	GO111MODULE=auto go mod tidy -modfile 'vendor.mod' -compat 1.17
-	GO111MODULE=auto go mod vendor -modfile vendor.mod
-fi
-
-if [ $# -eq 0 ] || [ "$1" = "archive/tar" ]; then
-	echo "update vendored copy of archive/tar"
-	: "${GO_VERSION:=$(awk -F '[ =]' '$1 == "ARG" && $2 == "GO_VERSION" { print $3; exit }' ./Dockerfile)}"
-	rm -rf vendor/archive/tar
-	mkdir -p vendor/archive/tar
-	echo "downloading: https://golang.org/dl/go${GO_VERSION%.0}.src.tar.gz"
-	curl -fsSL "https://golang.org/dl/go${GO_VERSION%.0}.src.tar.gz" \
-		| tar --extract --gzip --directory=vendor/archive/tar --strip-components=4 go/src/archive/tar
-	patch --strip=4 --directory=vendor/archive/tar --input="$PWD/patches/0001-archive-tar-do-not-populate-user-group-names.patch"
-fi
-
+GO111MODULE=auto go mod tidy -modfile 'vendor.mod' -compat 1.17
+GO111MODULE=auto go mod vendor -modfile vendor.mod
diff --git a/patches/0001-archive-tar-do-not-populate-user-group-names.patch b/patches/0001-archive-tar-do-not-populate-user-group-names.patch
diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
@@ -403,12 +403,64 @@ func (compression *Compression) Extension() string {
 	return ""
 }
 
+// nosysFileInfo hides the system-dependent info of the wrapped FileInfo to
+// prevent tar.FileInfoHeader from introspecting it and potentially calling into
+// glibc.
+type nosysFileInfo struct {
+	os.FileInfo
+}
+
+func (fi nosysFileInfo) Sys() interface{} {
+	// A Sys value of type *tar.Header is safe as it is system-independent.
+	// The tar.FileInfoHeader function copies the fields into the returned
+	// header without performing any OS lookups.
+	if sys, ok := fi.FileInfo.Sys().(*tar.Header); ok {
+		return sys
+	}
+	return nil
+}
+
+// sysStat, if non-nil, populates hdr from system-dependent fields of fi.
+var sysStat func(fi os.FileInfo, hdr *tar.Header) error
+
+// FileInfoHeaderNoLookups creates a partially-populated tar.Header from fi.
+//
+// Compared to the archive/tar.FileInfoHeader function, this function is safe to
+// call from a chrooted process as it does not populate fields which would
+// require operating system lookups. It behaves identically to
+// tar.FileInfoHeader when fi is a FileInfo value returned from
+// tar.Header.FileInfo().
+//
+// When fi is a FileInfo for a native file, such as returned from os.Stat() and
+// os.Lstat(), the returned Header value differs from one returned from
+// tar.FileInfoHeader in the following ways. The Uname and Gname fields are not
+// set as OS lookups would be required to populate them. The AccessTime and
+// ChangeTime fields are not currently set (not yet implemented) although that
+// is subject to change. Callers which require the AccessTime or ChangeTime
+// fields to be zeroed should explicitly zero them out in the returned Header
+// value to avoid any compatibility issues in the future.
+func FileInfoHeaderNoLookups(fi os.FileInfo, link string) (*tar.Header, error) {
+	hdr, err := tar.FileInfoHeader(nosysFileInfo{fi}, link)
+	if err != nil {
+		return nil, err
+	}
+	if sysStat != nil {
+		return hdr, sysStat(fi, hdr)
+	}
+	return hdr, nil
+}
+
 // FileInfoHeader creates a populated Header from fi.
-// Compared to archive pkg this function fills in more information.
-// Also, regardless of Go version, this function fills file type bits (e.g. hdr.Mode |= modeISDIR),
-// which have been deleted since Go 1.9 archive/tar.
+//
+// Compared to the archive/tar package, this function fills in less information
+// but is safe to call from a chrooted process. The AccessTime and ChangeTime
+// fields are not set in the returned header, ModTime is truncated to one-second
+// precision, and the Uname and Gname fields are only set when fi is a FileInfo
+// value returned from tar.Header.FileInfo(). Also, regardless of Go version,
+// this function fills file type bits (e.g. hdr.Mode |= modeISDIR), which have
+// been deleted since Go 1.9 archive/tar.
 func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, error) {
-	hdr, err := tar.FileInfoHeader(fi, link)
+	hdr, err := FileInfoHeaderNoLookups(fi, link)
 	if err != nil {
 		return nil, err
 	}
@@ -418,9 +470,6 @@ func FileInfoHeader(name string, fi os.FileInfo, link string) (*tar.Header, erro
 	hdr.ChangeTime = time.Time{}
 	hdr.Mode = fillGo18FileTypeBits(int64(chmodTarEntry(os.FileMode(hdr.Mode))), fi)
 	hdr.Name = canonicalTarName(name, fi.IsDir())
-	if err := setHeaderForSpecialDevice(hdr, name, fi.Sys()); err != nil {
-		return nil, err
-	}
 	return hdr, nil
 }
 
@@ -680,6 +729,7 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L
 		}
 
 	case tar.TypeLink:
+		//#nosec G305 -- The target path is checked for path traversal.
 		targetPath := filepath.Join(extractDir, hdr.Linkname)
 		// check for hardlink breakout
 		if !strings.HasPrefix(targetPath, extractDir) {
@@ -692,7 +742,7 @@ func createTarFile(path, extractDir string, hdr *tar.Header, reader io.Reader, L
 	case tar.TypeSymlink:
 		// 	path 				-> hdr.Linkname = targetPath
 		// e.g. /extractDir/path/to/symlink 	-> ../2/file	= /extractDir/path/2/file
-		targetPath := filepath.Join(filepath.Dir(path), hdr.Linkname)
+		targetPath := filepath.Join(filepath.Dir(path), hdr.Linkname) //#nosec G305 -- The target path is checked for path traversal.
 
 		// the reason we don't need to check symlinks in the path (with FollowSymlinkInScope) is because
 		// that symlink would first have to be created, which would be caught earlier, at this very check:
@@ -1045,6 +1095,7 @@ loop:
 			}
 		}
 
+		//#nosec G305 -- The joined path is checked for path traversal.
 		path := filepath.Join(dest, hdr.Name)
 		rel, err := filepath.Rel(dest, path)
 		if err != nil {
@@ -1109,6 +1160,7 @@ loop:
 	}
 
 	for _, hdr := range dirs {
+		//#nosec G305 -- The header was checked for path traversal before it was appended to the dirs slice.
 		path := filepath.Join(dest, hdr.Name)
 
 		if err := system.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil {
@@ -1251,7 +1303,7 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (err error) {
 			}
 			defer srcF.Close()
 
-			hdr, err := tar.FileInfoHeader(srcSt, "")
+			hdr, err := FileInfoHeaderNoLookups(srcSt, "")
 			if err != nil {
 				return err
 			}

diff --git a/pkg/archive/archive_linux.go b/pkg/archive/archive_linux.go
@@ -59,7 +59,7 @@ func (overlayWhiteoutConverter) ConvertWrite(hdr *tar.Header, path string, fi os
 				Gname:      hdr.Gname,
 				AccessTime: hdr.AccessTime,
 				ChangeTime: hdr.ChangeTime,
-			}
+			} //#nosec G305 -- An archive is being created, not extracted.
 		}
 	}
 

diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go
@@ -1402,3 +1402,12 @@ func TestPigz(t *testing.T) {
 		assert.Equal(t, reflect.TypeOf(contextReaderCloserWrapper.Reader), reflect.TypeOf(&gzip.Reader{}))
 	}
 }
+
+func TestNosysFileInfo(t *testing.T) {
+	st, err := os.Stat("archive_test.go")
+	assert.NilError(t, err)
+	h, err := tar.FileInfoHeader(nosysFileInfo{st}, "")
+	assert.NilError(t, err)
+	assert.Check(t, h.Uname == "")
+	assert.Check(t, h.Gname == "")
+}
diff --git a/pkg/archive/archive_unix.go b/pkg/archive/archive_unix.go
@@ -17,6 +17,10 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+func init() {
+	sysStat = statUnix
+}
+
 // fixVolumePathPrefix does platform specific processing to ensure that if
 // the path being passed in is not in a volume path format, convert it to one.
 func fixVolumePathPrefix(srcPath string) string {
@@ -45,19 +49,24 @@ func chmodTarEntry(perm os.FileMode) os.FileMode {
 	return perm // noop for unix as golang APIs provide perm bits correctly
 }
 
-func setHeaderForSpecialDevice(hdr *tar.Header, name string, stat interface{}) (err error) {
-	s, ok := stat.(*syscall.Stat_t)
+// statUnix populates hdr from system-dependent fields of fi without performing
+// any OS lookups.
+func statUnix(fi os.FileInfo, hdr *tar.Header) error {
+	s, ok := fi.Sys().(*syscall.Stat_t)
+	if !ok {
+		return nil
+	}
 
-	if ok {
-		// Currently go does not fill in the major/minors
-		if s.Mode&unix.S_IFBLK != 0 ||
-			s.Mode&unix.S_IFCHR != 0 {
-			hdr.Devmajor = int64(unix.Major(uint64(s.Rdev))) //nolint: unconvert
-			hdr.Devminor = int64(unix.Minor(uint64(s.Rdev))) //nolint: unconvert
-		}
+	hdr.Uid = int(s.Uid)
+	hdr.Gid = int(s.Gid)
+
+	if s.Mode&unix.S_IFBLK != 0 ||
+		s.Mode&unix.S_IFCHR != 0 {
+		hdr.Devmajor = int64(unix.Major(uint64(s.Rdev))) //nolint: unconvert
+		hdr.Devminor = int64(unix.Minor(uint64(s.Rdev))) //nolint: unconvert
 	}
 
-	return
+	return nil
 }
 
 func getInodeFromStat(stat interface{}) (inode uint64, err error) {

diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go
@@ -113,6 +113,7 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64,
 				continue
 			}
 		}
+		//#nosec G305 -- The joined path is guarded against path traversal.
 		path := filepath.Join(dest, hdr.Name)
 		rel, err := filepath.Rel(dest, path)
 		if err != nil {
@@ -209,6 +210,7 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64,
 	}
 
 	for _, hdr := range dirs {
+		//#nosec G305 -- The header was checked for path traversal before it was appended to the dirs slice.
 		path := filepath.Join(dest, hdr.Name)
 		if err := system.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil {
 			return 0, err

diff --git a/pkg/containerfs/archiver.go b/pkg/containerfs/archiver.go
@@ -137,7 +137,7 @@ func (archiver *Archiver) CopyFileWithTar(src, dst string) (retErr error) {
 			}
 			defer srcF.Close()
 
-			hdr, err := tar.FileInfoHeader(srcSt, "")
+			hdr, err := archive.FileInfoHeaderNoLookups(srcSt, "")
 			if err != nil {
 				return err
 			}

diff --git a/pkg/tarsum/tarsum.go b/pkg/tarsum/tarsum.go
@@ -246,6 +246,7 @@ func (ts *tarSum) Read(buf []byte) (int, error) {
 				return 0, err
 			}
 
+			//#nosec G305 -- The joined path is not passed to any filesystem APIs.
 			ts.currentFile = path.Join(".", path.Join("/", currentHeader.Name))
 			if err := ts.encodeHeader(currentHeader); err != nil {
 				return 0, err