feat(misconf): support symlinks inside of Helm archives (#6621)

pkg/iac/scanners/helm/parser/parser.go

@@ -22,7 +22,7 @@ import (
 	"helm.sh/helm/v3/pkg/releaseutil"
 
 	"github.com/aquasecurity/trivy/pkg/iac/debug"
-	detection2 "github.com/aquasecurity/trivy/pkg/iac/detection"
+	"github.com/aquasecurity/trivy/pkg/iac/detection"
 	"github.com/aquasecurity/trivy/pkg/iac/scanners/options"
 )
 
@@ -133,7 +133,7 @@ func (p *Parser) ParseFS(ctx context.Context, target fs.FS, path string) error {
 			return nil
 		}
 
-		if detection2.IsArchive(path) {
+		if detection.IsArchive(path) {
 			tarFS, err := p.addTarToFS(path)
 			if errors.Is(err, errSkipFS) {
 				// an unpacked Chart already exists
@@ -320,5 +320,5 @@ func (p *Parser) required(path string, workingFS fs.FS) bool {
 		return false
 	}
 
-	return detection2.IsType(path, bytes.NewReader(content), detection2.FileTypeHelm)
+	return detection.IsType(path, bytes.NewReader(content), detection.FileTypeHelm)
 }

pkg/iac/scanners/helm/parser/parser_tar.go

@@ -2,13 +2,13 @@ package parser
 
 import (
 	"archive/tar"
-	"bytes"
 	"compress/gzip"
 	"errors"
 	"fmt"
 	"io"
 	"io/fs"
 	"os"
+	"path"
 	"path/filepath"
 
 	"github.com/liamg/memoryfs"
@@ -18,18 +18,18 @@ import (
 
 var errSkipFS = errors.New("skip parse FS")
 
-func (p *Parser) addTarToFS(path string) (fs.FS, error) {
+func (p *Parser) addTarToFS(archivePath string) (fs.FS, error) {
 	tarFS := memoryfs.CloneFS(p.workingFS)
 
-	file, err := tarFS.Open(path)
+	file, err := tarFS.Open(archivePath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open tar: %w", err)
 	}
 	defer file.Close()
 
 	var tr *tar.Reader
 
-	if detection.IsZip(path) {
+	if detection.IsZip(archivePath) {
 		zipped, err := gzip.NewReader(file)
 		if err != nil {
 			return nil, fmt.Errorf("failed to create gzip reader: %w", err)
@@ -41,6 +41,7 @@ func (p *Parser) addTarToFS(path string) (fs.FS, error) {
 	}
 
 	checkExistedChart := true
+	symlinks := make(map[string]string)
 
 	for {
 		header, err := tr.Next()
@@ -51,61 +52,124 @@ func (p *Parser) addTarToFS(path string) (fs.FS, error) {
 			return nil, fmt.Errorf("failed to get next entry: %w", err)
 		}
 
+		name := filepath.ToSlash(header.Name)
+
 		if checkExistedChart {
 			// Do not add archive files to FS if the chart already exists
 			// This can happen when the source chart is located next to an archived chart (with the `helm package` command)
 			// The first level folder in the archive is equal to the Chart name
-			if _, err := tarFS.Stat(filepath.Dir(path) + "/" + filepath.Dir(header.Name)); err == nil {
+			if _, err := tarFS.Stat(path.Dir(archivePath) + "/" + path.Dir(name)); err == nil {
 				return nil, errSkipFS
 			}
 			checkExistedChart = false
 		}
 
 		// get the individual path and extract to the current directory
-		entryPath := header.Name
+		targetPath := path.Join(path.Dir(archivePath), path.Clean(name))
+
+		link := filepath.ToSlash(header.Linkname)
 
 		switch header.Typeflag {
 		case tar.TypeDir:
-			if err := tarFS.MkdirAll(entryPath, os.FileMode(header.Mode)); err != nil && !errors.Is(err, fs.ErrExist) {
+			if err := tarFS.MkdirAll(targetPath, os.FileMode(header.Mode)); err != nil && !errors.Is(err, fs.ErrExist) {
 				return nil, err
 			}
 		case tar.TypeReg:
-			writePath := filepath.Dir(path) + "/" + entryPath
-			p.debug.Log("Unpacking tar entry %s", writePath)
-
-			_ = tarFS.MkdirAll(filepath.Dir(writePath), fs.ModePerm)
-
-			buf, err := copyChunked(tr, 1024)
-			if err != nil {
+			p.debug.Log("Unpacking tar entry %s", targetPath)
+			if err := copyFile(tarFS, tr, targetPath); err != nil {
 				return nil, err
 			}
-
-			p.debug.Log("writing file contents to %s", writePath)
-			if err := tarFS.WriteFile(writePath, buf.Bytes(), fs.ModePerm); err != nil {
-				return nil, fmt.Errorf("write file error: %w", err)
+		case tar.TypeSymlink:
+			if path.IsAbs(link) {
+				p.debug.Log("Symlink %s is absolute, skipping", link)
+				continue
 			}
+
+			symlinks[targetPath] = path.Join(path.Dir(targetPath), link) // nolint:gosec // virtual file system is used
 		default:
 			return nil, fmt.Errorf("header type %q is not supported", header.Typeflag)
 		}
 	}
 
-	if err := tarFS.Remove(path); err != nil {
-		return nil, fmt.Errorf("failed to remove tar from FS: %w", err)
+	for target, link := range symlinks {
+		if err := copySymlink(tarFS, link, target); err != nil {
+			return nil, fmt.Errorf("copy symlink error: %w", err)
+		}
+	}
+
+	if err := tarFS.Remove(archivePath); err != nil {
+		return nil, fmt.Errorf("remove tar from FS error: %w", err)
 	}
 
 	return tarFS, nil
 }
 
-func copyChunked(src io.Reader, chunkSize int64) (*bytes.Buffer, error) {
-	buf := new(bytes.Buffer)
-	for {
-		if _, err := io.CopyN(buf, src, chunkSize); err != nil {
-			if errors.Is(err, io.EOF) {
-				break
-			}
-			return nil, fmt.Errorf("failed to copy: %w", err)
+func copySymlink(fsys *memoryfs.FS, src, dst string) error {
+	fi, err := fsys.Stat(src)
+	if err != nil {
+		return nil
+	}
+	if fi.IsDir() {
+		if err := copyDir(fsys, src, dst); err != nil {
+			return fmt.Errorf("copy dir error: %w", err)
+		}
+		return nil
+	}
+
+	if err := copyFileLazy(fsys, src, dst); err != nil {
+		return fmt.Errorf("copy file error: %w", err)
+	}
+
+	return nil
+}
+
+func copyFile(fsys *memoryfs.FS, src io.Reader, dst string) error {
+	if err := fsys.MkdirAll(path.Dir(dst), fs.ModePerm); err != nil && !errors.Is(err, fs.ErrExist) {
+		return fmt.Errorf("mkdir error: %w", err)
+	}
+
+	b, err := io.ReadAll(src)
+	if err != nil {
+		return fmt.Errorf("read error: %w", err)
+	}
+
+	if err := fsys.WriteFile(dst, b, fs.ModePerm); err != nil {
+		return fmt.Errorf("write file error: %w", err)
+	}
+
+	return nil
+}
+
+func copyDir(fsys *memoryfs.FS, src, dst string) error {
+	walkFn := func(filePath string, entry fs.DirEntry, err error) error {
+		if err != nil {
+			return err
 		}
+
+		if entry.IsDir() {
+			return nil
+		}
+
+		dst := path.Join(dst, filePath[len(src):])
+
+		if err := copyFileLazy(fsys, filePath, dst); err != nil {
+			return fmt.Errorf("copy file error: %w", err)
+		}
+		return nil
 	}
 
-	return buf, nil
+	return fs.WalkDir(fsys, src, walkFn)
+}
+
+func copyFileLazy(fsys *memoryfs.FS, src, dst string) error {
+	if err := fsys.MkdirAll(path.Dir(dst), fs.ModePerm); err != nil && !errors.Is(err, fs.ErrExist) {
+		return fmt.Errorf("mkdir error: %w", err)
+	}
+	return fsys.WriteLazyFile(dst, func() (io.Reader, error) {
+		f, err := fsys.Open(src)
+		if err != nil {
+			return nil, err
+		}
+		return f, nil
+	}, fs.ModePerm)
 }

pkg/iac/scanners/helm/parser/parser_test.go

@@ -22,4 +22,30 @@ func TestParseFS(t *testing.T) {
 		}
 		assert.Equal(t, expectedFiles, p.filepaths)
 	})
+
+	t.Run("archive with symlinks", func(t *testing.T) {
+		// mkdir -p chart && cd $_
+		// touch Chart.yaml
+		// mkdir -p dir && cp -p Chart.yaml dir/Chart.yaml
+		// mkdir -p sym-to-file && ln -s ../Chart.yaml sym-to-file/Chart.yaml
+		// ln -s dir sym-to-dir
+		// mkdir rec-sym && touch rec-sym/Chart.yaml
+		// ln -s . ./rec-sym/a
+		// cd .. && tar -czvf chart.tar.gz chart && rm -rf chart
+		p, err := New(".")
+		require.NoError(t, err)
+
+		fsys := os.DirFS(filepath.Join("testdata", "archive-with-symlinks"))
+		require.NoError(t, p.ParseFS(context.TODO(), fsys, "chart.tar.gz"))
+
+		expectedFiles := []string{
+			"chart/Chart.yaml",
+			"chart/dir/Chart.yaml",
+			"chart/rec-sym/Chart.yaml",
+			"chart/rec-sym/a/Chart.yaml",
+			"chart/sym-to-dir/Chart.yaml",
+			"chart/sym-to-file/Chart.yaml",
+		}
+		assert.Equal(t, expectedFiles, p.filepaths)
+	})
 }

pkg/iac/scanners/helm/test/parser_test.go

@@ -111,16 +111,14 @@ func Test_tar_is_chart(t *testing.T) {
 	}
 
 	for _, test := range tests {
+		t.Run(test.testName, func(t *testing.T) {
+			testPath := filepath.Join("testdata", test.archiveFile)
+			file, err := os.Open(testPath)
+			require.NoError(t, err)
+			defer file.Close()
 
-		t.Logf("Running test: %s", test.testName)
-		testPath := filepath.Join("testdata", test.archiveFile)
-		file, err := os.Open(testPath)
-		defer func() { _ = file.Close() }()
-		require.NoError(t, err)
-
-		assert.Equal(t, test.isHelmChart, detection.IsHelmChartArchive(test.archiveFile, file))
-
-		_ = file.Close()
+			assert.Equal(t, test.isHelmChart, detection.IsHelmChartArchive(test.archiveFile, file))
+		})
 	}
 }