Browse Source

pkg/archive: audit gosec file-traversal lints

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>
Cory Snider 3 years ago
parent
commit
833139f390
4 changed files with 8 additions and 2 deletions
  1. 4 1
      pkg/archive/archive.go
  2. 1 1
      pkg/archive/archive_linux.go
  3. 2 0
      pkg/archive/diff.go
  4. 1 0
      pkg/tarsum/tarsum.go

+ 4 - 1
pkg/archive/archive.go

@@ -729,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) {
@@ -741,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:
@@ -1094,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 {
@@ -1158,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 {

+ 1 - 1
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.
 		}
 	}
 

+ 2 - 0
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

+ 1 - 0
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