use tar.Header references over copies

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
anchore · Jul 11, 2024 · a6bb560 · a6bb560
1 parent 27b66b7
commit a6bb560
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 38 deletions.
diff --git a/pkg/file/metadata.go b/pkg/file/metadata.go
@@ -62,7 +62,7 @@ func (m ManualInfo) Sys() any {
 	return m.SysValue
 }
 
-func NewMetadata(header tar.Header, content io.Reader) Metadata {
+func NewMetadata(header *tar.Header, content io.Reader) Metadata {
 	return Metadata{
 		FileInfo:        header.FileInfo(),
 		Path:            path.Clean(DirSeparator + header.Name),

diff --git a/pkg/file/mime_type.go b/pkg/file/mime_type.go
@@ -15,7 +15,10 @@ func MIMEType(reader io.Reader) string {
 		return ""
 	}
 
-	s := sizer{reader: reader}
+	// though there is mimetype.SetLimit() at our disposal, that is a static resource which could be set by another
+	// library. To avoid any potential conflicts, we'll limit the reader ourselves. This is more of a safety measure
+	// to prevent performance regression than it is a performance optimization.
+	s := sizer{reader: io.LimitReader(reader, 3072)}
 
 	var mTypeStr string
 	mType, err := mimetype.DetectReader(&s)

diff --git a/pkg/file/tar_index.go b/pkg/file/tar_index.go
@@ -36,7 +36,6 @@ func NewTarIndex(tarFilePath string, onIndex TarIndexVisitor) (*TarIndex, error)
 		// body payload starts (after the header has been read).
 		indexEntry := TarIndexEntry{
 			path:         tarFileHandle.Name(),
-			sequence:     entry.Sequence,
 			header:       entry.Header,
 			seekPosition: entrySeekPosition,
 		}

diff --git a/pkg/file/tar_index_entry.go b/pkg/file/tar_index_entry.go
@@ -7,16 +7,14 @@ import (
 
 type TarIndexEntry struct {
 	path         string
-	sequence     int64
-	header       tar.Header
+	header       *tar.Header
 	seekPosition int64
 }
 
 func (t *TarIndexEntry) ToTarFileEntry() TarFileEntry {
 	return TarFileEntry{
-		Sequence: t.sequence,
-		Header:   t.header,
-		Reader:   t.Open(),
+		Header: t.header,
+		Reader: t.Open(),
 	}
 }
 

diff --git a/pkg/file/tarutil.go b/pkg/file/tarutil.go
@@ -26,9 +26,8 @@ type tarFile struct {
 
 // TarFileEntry represents the header, contents, and list position of an entry within a tar file.
 type TarFileEntry struct {
-	Sequence int64
-	Header   tar.Header
-	Reader   io.Reader
+	Header *tar.Header
+	Reader io.Reader
 }
 
 // TarFileVisitor is a visitor function meant to be used in conjunction with the IterateTar.
@@ -48,10 +47,7 @@ func (e *ErrFileNotFound) Error() string {
 // or if the visitor function returns a ErrTarStopIteration sentinel error.
 func IterateTar(reader io.Reader, visitor TarFileVisitor) error {
 	tarReader := tar.NewReader(reader)
-	var sequence int64 = -1
 	for {
-		sequence++
-
 		hdr, err := tarReader.Next()
 		if errors.Is(err, io.EOF) {
 			break
@@ -64,9 +60,8 @@ func IterateTar(reader io.Reader, visitor TarFileVisitor) error {
 		}
 
 		if err := visitor(TarFileEntry{
-			Sequence: sequence,
-			Header:   *hdr,
-			Reader:   tarReader,
+			Header: hdr,
+			Reader: tarReader,
 		}); err != nil {
 			if errors.Is(err, ErrTarStopIteration) {
 				return nil
@@ -144,7 +139,7 @@ type tarVisitor struct {
 }
 
 func (v tarVisitor) visit(entry TarFileEntry) error {
-	target := filepath.Join(v.destination, entry.Header.Name)
+	target := filepath.Join(v.destination, entry.Header.Name) //nolint:gosec  // we are checking path traversal issues just below this line
 
 	// we should not allow for any destination path to be outside of where we are unarchiving to
 	// "." is a special case that we allow (it is the root of the unarchived content)

diff --git a/pkg/file/tarutil_test.go b/pkg/file/tarutil_test.go
@@ -249,8 +249,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "regular file is written",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeReg,
 					Name:     "file.txt",
 					Linkname: "",
@@ -268,8 +267,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "regular file with possible path traversal errors out",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeReg,
 					Name:     "../file.txt",
 					Linkname: "",
@@ -282,8 +280,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "local . index is not a traversal error and should skip",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeDir,
 					Name:     ".",
 					Linkname: "",
@@ -296,8 +293,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "regular file with possible path traversal errors out (same prefix)",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeReg,
 					Name:     "../tmp-file.txt",
 					Linkname: "",
@@ -310,8 +306,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "directory is created",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeDir,
 					Name:     "dir",
 					Linkname: "",
@@ -327,8 +322,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "symlink is ignored",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeSymlink,
 					Name:     "symlink",
 					Linkname: "./../to-location",
@@ -344,8 +338,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "hardlink is ignored",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeLink,
 					Name:     "link",
 					Linkname: "./../to-location",
@@ -361,8 +354,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "device is ignored",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeChar,
 					Name:     "device",
 				},
@@ -377,8 +369,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "block device is ignored",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeBlock,
 					Name:     "device",
 				},
@@ -393,8 +384,7 @@ func Test_tarVisitor_visit(t *testing.T) {
 		{
 			name: "pipe is ignored",
 			entry: TarFileEntry{
-				Sequence: 0,
-				Header: tar.Header{
+				Header: &tar.Header{
 					Typeflag: tar.TypeFifo,
 					Name:     "pipe",
 				},