From 833139f3908f55c0c8c592910b8a2de01868ca1c Mon Sep 17 00:00:00 2001
From: Cory Snider <csnider@mirantis.com>
Date: Fri, 18 Feb 2022 15:30:24 -0500
Subject: [PATCH] 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>
---
 pkg/archive/archive.go       | 5 ++++-
 pkg/archive/archive_linux.go | 2 +-
 pkg/archive/diff.go          | 2 ++
 pkg/tarsum/tarsum.go         | 1 +
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go
index 1fc08ce98b..a9fd1e9552 100644
--- a/pkg/archive/archive.go
+++ b/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 {
diff --git a/pkg/archive/archive_linux.go b/pkg/archive/archive_linux.go
index 0a3cc1f92b..76321a35e3 100644
--- a/pkg/archive/archive_linux.go
+++ b/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.
 		}
 	}
 
diff --git a/pkg/archive/diff.go b/pkg/archive/diff.go
index e095104bd9..6174bc2af4 100644
--- a/pkg/archive/diff.go
+++ b/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
diff --git a/pkg/tarsum/tarsum.go b/pkg/tarsum/tarsum.go
index 5542e1b2c0..5ea65f1ecd 100644
--- a/pkg/tarsum/tarsum.go
+++ b/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