diff --git a/tar/extractor.go b/tar/extractor.go index 9ff62a931..89f6fa919 100644 --- a/tar/extractor.go +++ b/tar/extractor.go @@ -6,7 +6,7 @@ import ( "fmt" "io" "os" - fp "path/filepath" + "path/filepath" "runtime" "strings" "time" @@ -83,7 +83,7 @@ func (te *Extractor) Extract(reader io.Reader) error { rootName := header.Name // Get the platform-specific output path - rootOutputPath := fp.Clean(te.Path) + rootOutputPath := filepath.Clean(te.Path) if err := validatePlatformPath(rootOutputPath); err != nil { return err } @@ -131,7 +131,7 @@ func (te *Extractor) Extract(reader io.Reader) error { } // If the output path directory exists then put the file/symlink into the directory. - outputPath = fp.Join(rootOutputPath, rootName) + outputPath = filepath.Join(rootOutputPath, rootName) } // If an object with the target name already exists overwrite it @@ -142,7 +142,7 @@ func (te *Extractor) Extract(reader io.Reader) error { if err := updateMeta(outputPath, header.Mode, header.ModTime); err != nil { return err } - } else if err := te.extractSymlink(outputPath, header); err != nil { + } else if err := te.extractSymlink(outputPath, rootOutputPath, header); err != nil { return err } default: @@ -183,11 +183,11 @@ func (te *Extractor) Extract(reader io.Reader) error { // This check should already be covered by previous validation, but may catch bugs that slip through. // Checks if the relative path matches or exceeds the root // We check for matching because the outputPath function strips the original root - rel, err := fp.Rel(rootOutputPath, outputPath) + rel, err := filepath.Rel(rootOutputPath, outputPath) if err != nil || rel == "." { return errInvalidRootMultipleRoots } - for _, e := range strings.Split(fp.ToSlash(rel), "/") { + for _, e := range strings.Split(filepath.ToSlash(rel), "/") { if e == ".." { return errors.New("relative path contains '..'") } @@ -209,7 +209,7 @@ func (te *Extractor) Extract(reader io.Reader) error { return err } case tar.TypeSymlink: - if err := te.extractSymlink(outputPath, header); err != nil { + if err := te.extractSymlink(outputPath, rootOutputPath, header); err != nil { return err } default: @@ -257,7 +257,7 @@ func (te *Extractor) outputPath(basePlatformPath, relativeTarPath string) (strin if err := validatePathComponent(e); err != nil { return "", err } - platformPath = fp.Join(platformPath, e) + platformPath = filepath.Join(platformPath, e) // Last element is not checked since it will be removed (if it exists) by any of the extraction functions. // For more details see: @@ -290,20 +290,25 @@ func (te *Extractor) extractDir(path string) error { return err } - if stat, err := os.Lstat(path); err != nil { + stat, err := os.Lstat(path) + if err != nil { return err - } else if !stat.IsDir() { + } + if !stat.IsDir() { return errExtractedDirToSymlink } return nil } -func (te *Extractor) extractSymlink(path string, h *tar.Header) error { +func (te *Extractor) extractSymlink(path, rootPath string, h *tar.Header) error { err := os.Remove(path) if err != nil && !errors.Is(err, os.ErrNotExist) { return err } + // Before extracting a file or other symlink, the old path is removed to + // prevent a simlink being created that causes a subsequent extraction to + // escape the root. err = os.Symlink(h.Linkname, path) if err != nil { return err @@ -324,9 +329,10 @@ func (te *Extractor) extractFile(path string, r *tar.Reader) error { return err } - // Create a temporary file in the target directory and then rename the temporary file to the target to better deal - // with races on the file system. - base := fp.Dir(path) + // Create a temporary file in the target directory and then rename the + // temporary file to the target to better deal with races on the file + // system. + base := filepath.Dir(path) tmpfile, err := os.CreateTemp(base, "") if err != nil { return err diff --git a/tar/extractor_test.go b/tar/extractor_test.go index c8aa5833e..7e31fbea5 100644 --- a/tar/extractor_test.go +++ b/tar/extractor_test.go @@ -496,8 +496,9 @@ func TestLastElementOverwrite(t *testing.T) { &fileTarEntry{path: "root/symlink", buf: []byte("overwrite content")}, }, func(t *testing.T, extractDir string) { - // Check that outside-ref still exists but has not been - // overwritten or truncated (still size the same). + // Check that outside-ref still exists but has not been overwritten + // or truncated (still size the same). The symlink itself have been + // overwritten by the extracted file. info, err := os.Stat(fp.Join(extractDir, "..", "outside-ref")) assert.NoError(t, err)