diff --git a/archive.go b/archive.go index 2bcadec..f428fd3 100644 --- a/archive.go +++ b/archive.go @@ -439,6 +439,82 @@ func (ta *tarAppender) addTarFile(srcPath, archivePath string) error { return nil } +// resolveArchivePath resolves intermediate symlinks in name using chroot-like +// semantics when os.Root cannot traverse them. The final path component is +// intentionally preserved because archive extraction may create or replace it. +// +// This is a compatibility workaround rather than the preferred long-term +// implementation. It resolves the path separately before the actual operation, +// so a concurrent filesystem change may cause the operation to affect a +// different path within root. The subsequent os.Root operation still confines +// the operation to root and prevents such a change from escaping it. +// +// Paths with missing components are supported. Existing symlinks are resolved, +// and any remaining nonexistent components are retained for later creation. +// +// This helper should eventually be replaced by handle-relative resolution and +// operations with resolve-in-root semantics, avoiding the resolution/use race +// and repeated path traversal. +func resolveArchivePath(root *os.Root, name string) (string, error) { + parent, base := filepath.Split(name) + if parent == "" { + return name, nil + } + + parent = filepath.Clean(parent) + + // Follow the final parent component: it is an intermediate component of name, + // and an absolute symlink there must trigger the resolve-in-root fallback. + _, statErr := root.Stat(parent) + switch { + case statErr == nil: + return name, nil + case !os.IsNotExist(statErr) && !isPathEscapes(statErr): + return "", statErr + } + + // Resolve the parent both to handle ENOENT from missing components or dangling + // symlinks, and to determine whether an os.Root breakout was caused by an + // absolute symlink. Relative symlink escapes preserve the original Stat error. + resolved, err := resolveFSRootPath(root.Name(), parent) + if err != nil { + return "", err + } + + if isPathEscapes(statErr) && (!resolved.followedAbsoluteLink || resolved.relativeEscapeBeforeAbsolute) { + return "", statErr + } + + relParent, err := filepath.Rel(root.Name(), resolved.path) + if err != nil { + return "", breakoutError(fmt.Errorf( + "could not make resolved parent %q relative to root %q: %w", + resolved.path, + root.Name(), + err, + )) + } + if relParent != "." && !filepath.IsLocal(relParent) { + return "", breakoutError(fmt.Errorf( + "resolved parent %q escapes root %q", + resolved.path, + root.Name(), + )) + } + + return filepath.Join(relParent, base), nil +} + +// resolveHardlinkTarget validates a POSIX hardlink target and resolves it to +// the native, root-relative filesystem path used for extraction. +func resolveHardlinkTarget(root *os.Root, linkname string) (string, error) { + cleaned := path.Clean(linkname) + if cleaned == "." || !filepath.IsLocal(cleaned) { + return "", breakoutError(fmt.Errorf("invalid hardlink target %q", linkname)) + } + return resolveArchivePath(root, filepath.FromSlash(cleaned)) +} + // createTarFile extracts a single tar entry into the given root. dstPath is the // root-relative path of the entry being extracted, in native (host-separator) // form so it can be passed directly to os.Root methods and fsRootPath. @@ -462,6 +538,15 @@ func createTarFile(root *os.Root, dstPath string, hdr *tar.Header, reader io.Rea // so use hdrInfo.Mode() (they differ for e.g. setuid bits) hdrInfo := hdr.FileInfo() + var hardlinkTarget string + if hdr.Typeflag == tar.TypeLink { + var err error + hardlinkTarget, err = resolveHardlinkTarget(root, hdr.Linkname) + if err != nil { + return err + } + } + switch hdr.Typeflag { case tar.TypeDir: // Create directory unless it already exists as one; merge in that case. @@ -511,13 +596,7 @@ func createTarFile(root *os.Root, dstPath string, hdr *tar.Header, reader io.Rea } case tar.TypeLink: - // Defence in depth: root.Link's containment is limited when - // dest is a volume root. - linkname := path.Clean(hdr.Linkname) - if linkname == "." || !filepath.IsLocal(linkname) { - return breakoutError(fmt.Errorf("invalid hardlink target %q", hdr.Linkname)) - } - if err := root.Link(filepath.FromSlash(linkname), dstPath); err != nil { + if err := root.Link(hardlinkTarget, dstPath); err != nil { return err } @@ -593,7 +672,7 @@ func createTarFile(root *os.Root, dstPath string, hdr *tar.Header, reader io.Rea // 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(root, dstPath, hdr, hdrInfo); err != nil { + if err := handleLChmod(root, dstPath, hardlinkTarget, hdr, hdrInfo); err != nil { return err } @@ -608,7 +687,7 @@ func createTarFile(root *os.Root, dstPath string, hdr *tar.Header, reader io.Rea } case tar.TypeLink: // Follow the hardlink only when its target is not itself a symlink. - fi, err := root.Lstat(filepath.FromSlash(path.Clean(hdr.Linkname))) + fi, err := root.Lstat(hardlinkTarget) if err == nil && fi.Mode()&os.ModeSymlink == 0 { if err := chtimes(root, dstPath, aTime, mTime); err != nil { return err @@ -931,7 +1010,10 @@ loop: // dstPath is the native (host-separator) form of the entry name, // used at all filesystem boundaries (os.Root methods, fsRootPath). // hdr.Name stays POSIX (forward-slash) for logical string checks. - dstPath := filepath.FromSlash(hdr.Name) + dstPath, err := resolveArchivePath(root, filepath.FromSlash(hdr.Name)) + if err != nil { + return err + } // If dstPath exists we almost always just want to remove and replace it. // The only exception is when it is a directory *and* the file from @@ -969,7 +1051,7 @@ loop: // // This must be done before whiteoutConverter.ConvertRead, which // may set xattrs on the directory or create whiteout files. - if err := createImpliedDirectories(root, hdr, options); err != nil { + if err := createImpliedDirectories(root, dstPath, options); err != nil { return err } @@ -1024,17 +1106,19 @@ func unrepresentableOnWindows(hdr *tar.Header) error { return nil } -// createImpliedDirectories will create all parent directories of the current path with default permissions, if they do -// not already exist. This is possible as the tar format supports 'implicit' directories, where their existence is -// defined by the paths of files in the tar, but there are no header entries for the directories themselves, and thus -// we most both create them and choose metadata like permissions. +// createImpliedDirectories creates all parent directories of dstPath with +// default permissions if they do not already exist. This is necessary because +// the tar format permits implicit directories whose existence is defined only +// by file paths, without corresponding directory headers from which metadata +// could be restored. // -// The caller must have normalized hdr.Name (no leading ".." components). -// All directory creation is performed via root so it is bounded within the -// destination at the OS level (openat(2) semantics), preventing escape via -// symlinks in the destination tree. -func createImpliedDirectories(root *os.Root, hdr *tar.Header, options *TarOptions) error { - parent := filepath.FromSlash(path.Dir(strings.TrimSuffix(hdr.Name, "/"))) +// The caller must pass a normalized, root-relative local path. Any archive-path +// conversion and resolve-in-root handling must already have been applied. +// Directory creation is performed through root, so it remains confined to the +// extraction destination even if the destination tree changes concurrently. +func createImpliedDirectories(root *os.Root, dstPath string, options *TarOptions) error { + parent := filepath.Dir(dstPath) + // Skip when the parent is the root itself; nothing to create. if parent == "." || parent == "" { return nil diff --git a/archive_unix.go b/archive_unix.go index e2dcc42..482ab69 100644 --- a/archive_unix.go +++ b/archive_unix.go @@ -8,7 +8,6 @@ import ( "fmt" "math" "os" - "path" "path/filepath" "strings" "syscall" @@ -88,7 +87,7 @@ func handleTarTypeBlockCharFifo(root *os.Root, hdr *tar.Header, dstPath string) // handleLChmod applies the mode from hdrInfo to dstPath within root, skipping // symlinks (there is no lchmod). For hardlinks, the mode is applied only when // the link target is itself not a symlink. -func handleLChmod(root *os.Root, dstPath string, hdr *tar.Header, hdrInfo os.FileInfo) error { +func handleLChmod(root *os.Root, dstPath string, hardlinkTarget string, hdr *tar.Header, hdrInfo os.FileInfo) error { switch hdr.Typeflag { case tar.TypeSymlink: return nil @@ -96,7 +95,7 @@ func handleLChmod(root *os.Root, dstPath string, hdr *tar.Header, hdrInfo os.Fil case tar.TypeLink: // If the target is a symlink, there is no way to chmod the hardlink // without following it. - fi, err := root.Lstat(filepath.FromSlash(path.Clean(hdr.Linkname))) + fi, err := root.Lstat(hardlinkTarget) if err != nil || fi.Mode()&os.ModeSymlink != 0 { return nil } diff --git a/archive_unix_test.go b/archive_unix_test.go index 841ae5e..a574c20 100644 --- a/archive_unix_test.go +++ b/archive_unix_test.go @@ -510,3 +510,207 @@ func TestHandleTarTypeBlockCharFifoDeviceRange(t *testing.T) { }) } } + +// TestUntarThroughAbsoluteSymlink verifies that archive extraction follows a +// pre-existing absolute symlink relative to the extraction root, including +// when the symlink target or directories following it do not yet exist. +// +// Regression test for https://github.com/moby/moby/issues/53258 +func TestUntarThroughAbsoluteSymlink(t *testing.T) { + unpackers := []struct { + name string + unpack func(dest string, r io.Reader) error + }{ + { + name: "Untar", + unpack: func(dest string, r io.Reader) error { + return Untar(r, dest, &TarOptions{NoLchown: true}) + }, + }, + { + name: "UnpackLayer", + unpack: func(dest string, r io.Reader) error { + _, err := UnpackLayer(dest, r, &TarOptions{NoLchown: true}) + return err + }, + }, + } + + for _, unpacker := range unpackers { + t.Run(unpacker.name, func(t *testing.T) { + for _, tc := range []struct { + name string + createTarget bool + }{ + { + name: "existing target", + createTarget: true, + }, + { + name: "missing target", + createTarget: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + const ( + name = "var/run/existing/non-existing/file" + content = "content" + ) + + dest := t.TempDir() + assert.NilError(t, os.Mkdir(filepath.Join(dest, "var"), 0o755)) + if tc.createTarget { + assert.NilError(t, os.MkdirAll( + filepath.Join(dest, "run", "existing"), + 0o755, + )) + } + assert.NilError(t, os.Symlink( + "/run", + filepath.Join(dest, "var", "run"), + )) + + buf := &bytes.Buffer{} + tw := tar.NewWriter(buf) + assert.NilError(t, tw.WriteHeader(&tar.Header{ + Name: name, + Typeflag: tar.TypeReg, + Mode: 0o644, + Size: int64(len(content)), + })) + _, err := io.WriteString(tw, content) + assert.NilError(t, err) + assert.NilError(t, tw.Close()) + + assert.NilError(t, unpacker.unpack(dest, buf)) + + actual, err := os.ReadFile(filepath.Join( + dest, "run", "existing", "non-existing", "file", + )) + assert.NilError(t, err) + assert.DeepEqual(t, actual, []byte(content)) + }) + } + }) + } +} + +// A relative symlink must not escape the extraction root merely because path +// resolution encounters an absolute symlink afterward. +func TestUnpackRejectsRelativeEscapeBeforeAbsoluteSymlink(t *testing.T) { + buf := &bytes.Buffer{} + tw := tar.NewWriter(buf) + assert.NilError(t, tw.WriteHeader(&tar.Header{ + Name: "escape/absolute/file", + Typeflag: tar.TypeReg, + Mode: 0o644, + })) + assert.NilError(t, tw.Close()) + + unpackers := []struct { + name string + unpack func(dest string, r io.Reader) error + }{ + { + name: "Unpack", + unpack: func(dest string, r io.Reader) error { + return Unpack(r, dest, &TarOptions{NoLchown: true}) + }, + }, + { + name: "UnpackLayer", + unpack: func(dest string, r io.Reader) error { + _, err := UnpackLayer(dest, r, &TarOptions{NoLchown: true}) + return err + }, + }, + } + + for _, unpacker := range unpackers { + t.Run(unpacker.name, func(t *testing.T) { + dest := t.TempDir() + assert.NilError(t, os.Mkdir(filepath.Join(dest, "target"), 0o755)) + assert.NilError(t, os.Symlink("..", filepath.Join(dest, "escape"))) + assert.NilError(t, os.Symlink( + "/target", + filepath.Join(dest, "absolute"), + )) + + err := unpacker.unpack(dest, bytes.NewReader(buf.Bytes())) + assert.Check(t, isPathEscapes(err), "expected path-escape error, got: %v", err) + + _, err = os.Lstat(filepath.Join(dest, "target", "file")) + assert.Check(t, os.IsNotExist(err), "archive wrote through rejected path: %v", err) + }) + } +} + +// Absolute symlinks are common in container root filesystems and may come from +// a lower layer. Later layers must resolve files and hardlink sources through +// those symlinks relative to the extraction root, not the host root. +func TestHardlinkSourceThroughAbsoluteSymlink(t *testing.T) { + const content = "content" + + unpackers := []struct { + name string + unpack func(io.Reader, string) error + }{ + { + name: "Unpack", + unpack: func(r io.Reader, dest string) error { + return Unpack(r, dest, &TarOptions{NoLchown: true}) + }, + }, + { + name: "UnpackLayer", + unpack: func(r io.Reader, dest string) error { + _, err := UnpackLayer(dest, r, &TarOptions{NoLchown: true}) + return err + }, + }, + } + + for _, tc := range unpackers { + t.Run(tc.name, func(t *testing.T) { + dest := t.TempDir() + assert.NilError(t, os.Mkdir(filepath.Join(dest, "var"), 0o755)) + assert.NilError(t, os.Symlink("/run", filepath.Join(dest, "var", "run"))) + + buf := &bytes.Buffer{} + tw := tar.NewWriter(buf) + assert.NilError(t, tw.WriteHeader(&tar.Header{ + Name: "var/run/source", + Typeflag: tar.TypeReg, + Mode: 0o644, + Size: int64(len(content)), + })) + _, err := io.WriteString(tw, content) + assert.NilError(t, err) + assert.NilError(t, tw.WriteHeader(&tar.Header{ + Name: "var/run/link", + Typeflag: tar.TypeLink, + Linkname: "var/run/source", + Mode: 0o644, + })) + assert.NilError(t, tw.Close()) + + assert.NilError(t, tc.unpack(buf, dest)) + + source := filepath.Join(dest, "run", "source") + link := filepath.Join(dest, "run", "link") + actual, err := os.ReadFile(link) + assert.NilError(t, err) + assert.DeepEqual(t, actual, []byte(content)) + + sourceInode, err := getInode(source) + assert.NilError(t, err) + linkInode, err := getInode(link) + assert.NilError(t, err) + assert.Equal(t, sourceInode, linkInode) + + linkCount, err := getNlink(source) + assert.NilError(t, err) + assert.Equal(t, linkCount, uint64(2)) + }) + } +} diff --git a/archive_windows.go b/archive_windows.go index ffe00a4..aa9e523 100644 --- a/archive_windows.go +++ b/archive_windows.go @@ -53,7 +53,7 @@ func handleTarTypeBlockCharFifo(root *os.Root, hdr *tar.Header, path string) err } // handleLChmod is a no-op on Windows because chmod is not supported. -func handleLChmod(root *os.Root, path string, hdr *tar.Header, hdrInfo os.FileInfo) error { +func handleLChmod(root *os.Root, dstPath string, hardlinkTarget string, hdr *tar.Header, hdrInfo os.FileInfo) error { return nil } diff --git a/diff.go b/diff.go index 055f3c1..b945018 100644 --- a/diff.go +++ b/diff.go @@ -29,8 +29,9 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, tr := tar.NewReader(layer) var dirs []unpackedDir - // unpackedPaths tracks root-relative paths already written in this layer - // so that the AUFS opaque-whiteout walk knows which paths to preserve. + // unpackedPaths tracks resolved, native-separator, root-relative paths + // already written in this layer so that the AUFS opaque-whiteout walk + // knows which paths to preserve. unpackedPaths := make(map[string]struct{}) if options == nil { @@ -71,12 +72,6 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, continue } - // Ensure that the parent directory exists. - err = createImpliedDirectories(root, hdr, options) - if err != nil { - return 0, err - } - // Skip AUFS metadata dirs if strings.HasPrefix(hdr.Name, WhiteoutMetaPrefix) { // Regular files inside /.wh..wh.plnk can be used as hardlink targets @@ -109,10 +104,15 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, // dstPath is the native (host-separator) form of the entry name, // used at all filesystem boundaries (os.Root methods, fsRootPath). // The tar-header name (hdr.Name) is POSIX, so convert it here. - dstPath := filepath.FromSlash(hdr.Name) - base := filepath.Base(dstPath) - - if strings.HasPrefix(base, WhiteoutPrefix) { + dstPath, err := resolveArchivePath(root, filepath.FromSlash(hdr.Name)) + if err != nil { + return 0, err + } + // Ensure that the parent directory exists. + if err := createImpliedDirectories(root, dstPath, options); err != nil { + return 0, err + } + if base := filepath.Base(dstPath); strings.HasPrefix(base, WhiteoutPrefix) { dir := filepath.Dir(dstPath) if base == WhiteoutOpaqueDir { _, err := root.Lstat(dir) @@ -144,9 +144,9 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, return err } - // unpackedPaths is keyed by root-relative slash paths; convert - // filepath.WalkDir's native path before looking it up. - if _, exists := unpackedPaths[filepath.ToSlash(rel)]; !exists { + // unpackedPaths is keyed by resolved, native-separator, + // root-relative paths, matching filepath.WalkDir's paths. + if _, exists := unpackedPaths[rel]; !exists { return root.RemoveAll(rel) } return nil @@ -206,9 +206,9 @@ func UnpackLayer(dest string, layer io.Reader, options *TarOptions) (size int64, if hdr.Typeflag == tar.TypeDir { dirs = append(dirs, unpackedDir{hdr: hdr, name: dstPath}) } - // unpackedPaths is keyed by the POSIX (forward-slash) name so it - // matches the ToSlash'd lookup in the opaque-whiteout walk above. - unpackedPaths[hdr.Name] = struct{}{} + // Record the resolved, native-separator, root-relative path so it + // matches the paths produced by the opaque-whiteout walk. + unpackedPaths[dstPath] = struct{}{} } } diff --git a/rootpath.go b/rootpath.go index 3834af2..7d34839 100644 --- a/rootpath.go +++ b/rootpath.go @@ -24,31 +24,47 @@ import ( var errTooManyLinks = errors.New("too many links") +type fsRootPathResult struct { + path string + followedAbsoluteLink bool + relativeEscapeBeforeAbsolute bool +} + // fsRootPath joins a path with a root, evaluating and bounding any // symlink to the root directory. func fsRootPath(root, path string) (string, error) { + result, err := resolveFSRootPath(root, path) + if err != nil { + return "", err + } + return result.path, nil +} + +func resolveFSRootPath(root, path string) (fsRootPathResult, error) { + result := fsRootPathResult{path: root} if path == "" { - return root, nil + return result, nil } var linksWalked int // to protect against cycles for { i := linksWalked - newpath, err := walkLinks(root, path, &linksWalked) + newpath, err := walkLinks(root, path, &linksWalked, &result) if err != nil { - return "", err + return fsRootPathResult{}, err } path = newpath if i == linksWalked { newpath = filepath.Join(string(os.PathSeparator), newpath) if path == newpath { - return filepath.Join(root, newpath), nil + result.path = filepath.Join(root, newpath) + return result, nil } path = newpath } } } -func walkLink(root, path string, linksWalked *int) (newpath string, islink bool, err error) { +func walkLink(root, path string, linksWalked *int, result *fsRootPathResult) (newpath string, islink bool, err error) { if *linksWalked > 255 { return "", false, errTooManyLinks } @@ -74,37 +90,51 @@ func walkLink(root, path string, linksWalked *int) (newpath string, islink bool, if err != nil { return "", false, err } + if filepath.IsAbs(newpath) { + result.followedAbsoluteLink = true + } else if !result.followedAbsoluteLink { + // Record an escape before a later absolute link can make the original + // os.Root error appear eligible for resolve-in-root fallback. + relativeDir, err := filepath.Rel(string(os.PathSeparator), filepath.Dir(path)) + if err != nil { + return "", false, err + } + + resolved := filepath.Join(relativeDir, newpath) + if resolved != "." && !filepath.IsLocal(resolved) { + result.relativeEscapeBeforeAbsolute = true + } + } + *linksWalked++ return newpath, true, nil } -func walkLinks(root, path string, linksWalked *int) (string, error) { +func walkLinks(root, path string, linksWalked *int, result *fsRootPathResult) (string, error) { switch dir, file := filepath.Split(path); { case dir == "": - newpath, _, err := walkLink(root, file, linksWalked) + newpath, _, err := walkLink(root, file, linksWalked, result) return newpath, err case file == "": if os.IsPathSeparator(dir[len(dir)-1]) { if dir == string(os.PathSeparator) { return dir, nil } - return walkLinks(root, dir[:len(dir)-1], linksWalked) + return walkLinks(root, dir[:len(dir)-1], linksWalked, result) } - newpath, _, err := walkLink(root, dir, linksWalked) + newpath, _, err := walkLink(root, dir, linksWalked, result) return newpath, err + default: - newdir, err := walkLinks(root, dir, linksWalked) + newdir, err := walkLinks(root, dir, linksWalked, result) if err != nil { return "", err } - newpath, islink, err := walkLink(root, filepath.Join(newdir, file), linksWalked) + newpath, islink, err := walkLink(root, filepath.Join(newdir, file), linksWalked, result) if err != nil { return "", err } - if !islink { - return newpath, nil - } - if filepath.IsAbs(newpath) { + if !islink || filepath.IsAbs(newpath) { return newpath, nil } return filepath.Join(newdir, newpath), nil