From 4eae37c52b035b3576361c12f70d3d9517d0a73c Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Wed, 8 May 2024 05:23:55 +0600 Subject: [PATCH] feat(misconf): support symlinks inside of Helm archives (#6621) --- pkg/iac/scanners/helm/parser/parser.go | 6 +- pkg/iac/scanners/helm/parser/parser_tar.go | 122 +++++++++++++----- pkg/iac/scanners/helm/parser/parser_test.go | 26 ++++ .../archive-with-symlinks/chart.tar.gz | Bin 0 -> 312 bytes pkg/iac/scanners/helm/test/parser_test.go | 16 +-- 5 files changed, 129 insertions(+), 41 deletions(-) create mode 100644 pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/chart.tar.gz diff --git a/pkg/iac/scanners/helm/parser/parser.go b/pkg/iac/scanners/helm/parser/parser.go index c8bc8a73bedd..ddcfac09682f 100644 --- a/pkg/iac/scanners/helm/parser/parser.go +++ b/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) } diff --git a/pkg/iac/scanners/helm/parser/parser_tar.go b/pkg/iac/scanners/helm/parser/parser_tar.go index 5455ab780683..f8d692eaa977 100644 --- a/pkg/iac/scanners/helm/parser/parser_tar.go +++ b/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,10 +18,10 @@ 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) } @@ -29,7 +29,7 @@ func (p *Parser) addTarToFS(path string) (fs.FS, error) { 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) } diff --git a/pkg/iac/scanners/helm/parser/parser_test.go b/pkg/iac/scanners/helm/parser/parser_test.go index 030b0efbb86f..9c8b05ce7696 100644 --- a/pkg/iac/scanners/helm/parser/parser_test.go +++ b/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) + }) } diff --git a/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/chart.tar.gz b/pkg/iac/scanners/helm/parser/testdata/archive-with-symlinks/chart.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..a3183710c17f5dd1d04935cfd2731450a83df2b5 GIT binary patch literal 312 zcmV-80muFyiwFSaw>f421MQbxYQr!LfO8aIAenz;`2ahKNgGyZC^+duZ$Bjt9a+nm zVKR*B3)n9vR{U9t-G9V1zcs9At%LV!?J@V-Lhd(|0W|2G3)(4doCnt^&l^_eI?XXr zDD!2aXN>9kd>joMh9BOueaF53C-kfTbnM&dHZtvlBL5D_zc)^c{~Bcf?@Qa=;&|qm zgVqB9e&-)?^$&=R(j38jNBooj!znTT14{BAgCYOmte5<+LH19zznw2FZ3B)u%KQiN zk0wz5Tc&t+i2pDD!|8lvr~_sGVX6PY1j+vzJo^9pvp}(bYeJySA+fik7o~*_|Nx$YQ6Zc!QOvlnWLosZ})$<