Skip to content

Commit

Permalink
pkg/archive: audit gosec file-traversal lints
Browse files Browse the repository at this point in the history
The recently-upgraded gosec linter has a rule for archive extraction
code which may be vulnerable to directory traversal attacks, a.k.a. Zip
Slip. Gosec's detection is unfortunately prone to false positives,
however: it flags any filepath.Join call with an argument derived from a
tar.Header value, irrespective of whether the resultant path is used for
filesystem operations or if directory traversal attacks are guarded
against.

All of the lint errors reported by gosec appear to be false positives.

Signed-off-by: Cory Snider <csnider@mirantis.com>
(cherry picked from commit 833139f)
Signed-off-by: Cory Snider <csnider@mirantis.com>
  • Loading branch information
corhere committed Oct 23, 2023
1 parent 8e44855 commit 31b8374
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion pkg/archive/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,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) {
Expand All @@ -690,7 +691,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:
Expand Down Expand Up @@ -1019,6 +1020,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 {
Expand Down Expand Up @@ -1083,6 +1085,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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/archive/archive_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/archive/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/tarsum/tarsum.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 31b8374

Please sign in to comment.