Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
"fmt"
"io"
"os"
"path"
"path/filepath"
"runtime"
"strings"
Expand Down Expand Up @@ -831,16 +832,19 @@
continue
}

// Normalize name, for safety and for a simple is-root check
// This keeps "../" as-is, but normalizes "/../" to "/". Or Windows:
// This keeps "..\" as-is, but normalizes "\..\" to "\".
hdr.Name = filepath.Clean(hdr.Name)

// Strip any leading "/" so absolute entries stay root-relative, and
// normalize the POSIX tar path. Skip entries referring to the extraction
// root and reject paths that escape it.
name := path.Clean(strings.TrimLeft(hdr.Name, "/"))
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
if name == "." {
continue
}
for _, exclude := range options.ExcludePatterns {
if strings.HasPrefix(hdr.Name, exclude) {
if strings.HasPrefix(name, exclude) {
continue loop
}

Check failure

Code scanning / CodeQL

Arbitrary file write extracting an archive containing symbolic links High

Unresolved path from an archive header, which may point outside the archive root, is used in
symlink creation
.
}
hdr.Name = name

// Ensure that the parent directory exists.
err = createImpliedDirectories(dest, hdr, options)
Expand All @@ -849,10 +853,10 @@
}

// #nosec G305 -- The joined path is checked for path traversal.
dstPath := filepath.Join(dest, hdr.Name)
dstPath := filepath.Join(dest, filepath.FromSlash(hdr.Name))
rel, err := filepath.Rel(dest, dstPath)
if err != nil {
return err
Comment on lines 855 to 859
}
if strings.HasPrefix(rel, ".."+string(os.PathSeparator)) {
return breakoutError(fmt.Errorf("%q is outside of %q", hdr.Name, dest))
Expand Down Expand Up @@ -913,7 +917,7 @@

for _, hdr := range dirs {
// #nosec G305 -- The header was checked for path traversal before it was appended to the dirs slice.
dstPath := filepath.Join(dest, hdr.Name)
dstPath := filepath.Join(dest, filepath.FromSlash(hdr.Name))
if err := chtimes(dstPath, boundTime(latestTime(hdr.AccessTime, hdr.ModTime)), boundTime(hdr.ModTime)); err != nil {
return err
}
Expand Down
37 changes: 26 additions & 11 deletions diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"fmt"
"io"
"os"
"path"
"path/filepath"
"runtime"
"strings"
Expand Down Expand Up @@ -48,8 +49,14 @@

size += hdr.Size

// Normalize name, for safety and for a simple is-root check
hdr.Name = filepath.Clean(hdr.Name)
// Strip any leading "/" so absolute entries stay root-relative, and
// normalize the POSIX tar path. Skip entries referring to the extraction
// root and reject paths that escape it.
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
name := path.Clean(strings.TrimLeft(hdr.Name, "/"))
if name == "." {
continue
}
hdr.Name = name
Comment on lines +52 to +59

// Windows does not support filenames with colons in them. Ignore
// these files. This is not a problem though (although it might
Expand Down Expand Up @@ -83,16 +90,20 @@
// Regular files inside /.wh..wh.plnk can be used as hardlink targets
// We don't want this directory, but we need the files in them so that
// such hardlinks can be resolved.
if strings.HasPrefix(hdr.Name, WhiteoutLinkDir) && hdr.Typeflag == tar.TypeReg {
basename := filepath.Base(hdr.Name)
if strings.HasPrefix(hdr.Name, WhiteoutLinkDir+"/") && hdr.Typeflag == tar.TypeReg {
basename := path.Base(hdr.Name)
localBasename, err := filepath.Localize(basename)
if err != nil || filepath.Base(localBasename) != localBasename {
return 0, breakoutError(fmt.Errorf("invalid AUFS hardlink name %q", hdr.Name))
}
aufsHardlinks[basename] = hdr
if aufsTempdir == "" {
if aufsTempdir, err = os.MkdirTemp(dest, "dockerplnk"); err != nil {
return 0, err
}
defer os.RemoveAll(aufsTempdir)
}
if err := createTarFile(filepath.Join(aufsTempdir, basename), dest, hdr, tr, options); err != nil {
if err := createTarFile(filepath.Join(aufsTempdir, localBasename), dest, hdr, tr, options); err != nil {
return 0, err
}
}
Expand All @@ -102,7 +113,7 @@
}
}
// #nosec G305 -- The joined path is guarded against path traversal.
dstPath := filepath.Join(dest, hdr.Name)
dstPath := filepath.Join(dest, filepath.FromSlash(hdr.Name))
rel, err := filepath.Rel(dest, dstPath)
if err != nil {
return 0, err
Comment on lines 115 to 119
Expand Down Expand Up @@ -164,13 +175,17 @@

// Hard links into /.wh..wh.plnk don't work, as we don't extract that directory, so
// we manually retarget these into the temporary files we extracted them into
if hdr.Typeflag == tar.TypeLink && strings.HasPrefix(filepath.Clean(hdr.Linkname), WhiteoutLinkDir) {
linkBasename := filepath.Base(hdr.Linkname)
if hdr.Typeflag == tar.TypeLink && strings.HasPrefix(path.Clean(hdr.Linkname), WhiteoutLinkDir+"/") {
linkBasename := path.Base(hdr.Linkname)
srcHdr = aufsHardlinks[linkBasename]
if srcHdr == nil {
return 0, errors.New("invalid aufs hardlink")
return 0, errors.New("invalid AUFS hardlink")
}
localBasename, err := filepath.Localize(linkBasename)
if err != nil || filepath.Base(localBasename) != localBasename {
return 0, breakoutError(fmt.Errorf("invalid AUFS hardlink name %q", hdr.Linkname))
}
tmpFile, err := os.Open(filepath.Join(aufsTempdir, linkBasename))
tmpFile, err := os.Open(filepath.Join(aufsTempdir, localBasename))
if err != nil {
return 0, err
}
Expand All @@ -197,7 +212,7 @@

for _, hdr := range dirs {
// #nosec G305 -- The header was checked for path traversal before it was appended to the dirs slice.
dstPath := filepath.Join(dest, hdr.Name)
dstPath := filepath.Join(dest, filepath.FromSlash(hdr.Name))
if err := chtimes(dstPath, hdr.AccessTime, hdr.ModTime); err != nil {
return 0, err
}
Expand Down
Loading