Protect template extraction from tar bombs and path traversal

internal/templaterepo/client.go

@@ -4,6 +4,7 @@ import (
 	"archive/tar"
 	"compress/gzip"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -26,6 +27,20 @@ const (
 	templateMetadataFile = ".cre/template.yaml"
 )
 
+// maxExtractTotalSize and maxExtractFileSize bound decompression (a "tar bomb"
+// can expand to far more bytes than the compressed download), and maxExtractFileCount
+// bounds the number of entries, mirroring the caps used by the update extractor.
+// maxTarballDownloadSize bounds the raw (still-compressed) tarball written to the
+// on-disk cache, guarding against disk exhaustion on the download side.
+// These are vars (not consts) so tests can shrink them instead of building
+// multi-hundred-megabyte fixtures.
+var (
+	maxExtractTotalSize    int64 = 500 * 1024 * 1024
+	maxExtractFileSize     int64 = 100 * 1024 * 1024
+	maxTarballDownloadSize int64 = 500 * 1024 * 1024
+	maxExtractFileCount          = 10000
+)
+
 // standardIgnores are files/dirs always excluded when extracting templates.
 var standardIgnores = []string{
 	".git",
@@ -253,9 +268,20 @@ func (c *Client) DownloadTarball(source RepoSource, destPath string) error {
 	}
 	defer f.Close()
 
-	if _, err := io.Copy(f, resp.Body); err != nil {
+	// Cap the raw download so a compromised or malicious source cannot exhaust
+	// disk by streaming an unbounded tarball into the cache.
+	written, err := io.CopyN(f, resp.Body, maxTarballDownloadSize+1)
+	if err != nil && !errors.Is(err, io.EOF) {
 		return fmt.Errorf("failed to write tarball: %w", err)
 	}
+	if written > maxTarballDownloadSize {
+		// Drop the partial file so it can't poison the cache.
+		f.Close()
+		if rmErr := os.Remove(destPath); rmErr != nil {
+			c.logger.Warn().Err(rmErr).Msgf("Failed to remove oversized partial tarball %s", destPath)
+		}
+		return fmt.Errorf("tarball exceeds maximum allowed download size (limit %d bytes)", maxTarballDownloadSize)
+	}
 
 	return nil
 }
@@ -346,9 +372,17 @@ func (c *Client) extractTarball(r io.Reader, templatePath, destDir string, exclu
 	// We need to detect it and strip it.
 	var topLevelPrefix string
 
+	absDestDir, err := filepath.Abs(destDir)
+	if err != nil {
+		return fmt.Errorf("failed to resolve destination directory: %w", err)
+	}
+
+	var totalSize int64
+	var fileCount int
+
 	for {
 		header, err := tr.Next()
-		if err == io.EOF {
+		if errors.Is(err, io.EOF) {
 			break
 		}
 		if err != nil {
@@ -365,6 +399,11 @@ func (c *Client) extractTarball(r io.Reader, templatePath, destDir string, exclu
 			return fmt.Errorf("illegal file path in archive: %s", header.Name)
 		}
 
+		fileCount++
+		if fileCount > maxExtractFileCount {
+			return fmt.Errorf("archive contains too many entries (limit: %d)", maxExtractFileCount)
+		}
+
 		// Detect top-level prefix from the first real directory entry
 		if topLevelPrefix == "" {
 			parts := strings.SplitN(header.Name, "/", 2)
@@ -410,6 +449,17 @@ func (c *Client) extractTarball(r io.Reader, templatePath, destDir string, exclu
 
 		targetPath := filepath.Join(destDir, relPath)
 
+		// Belt-and-suspenders Zip Slip guard: verify the resolved path is still
+		// contained within destDir, in case the ".." check above is bypassed by
+		// some other means (e.g. an absolute path in the header).
+		absTargetPath, err := filepath.Abs(targetPath)
+		if err != nil {
+			return fmt.Errorf("failed to resolve target path for %s: %w", name, err)
+		}
+		if absTargetPath != absDestDir && !strings.HasPrefix(absTargetPath, absDestDir+string(os.PathSeparator)) {
+			return fmt.Errorf("resolved file path escapes destination directory: %s", header.Name)
+		}
+
 		switch header.Typeflag {
 		case tar.TypeDir:
 			c.logger.Debug().Msgf("Extracting dir: %s -> %s", name, targetPath)
@@ -429,16 +479,30 @@ func (c *Client) extractTarball(r io.Reader, templatePath, destDir string, exclu
 				return fmt.Errorf("failed to create parent directory: %w", err)
 			}
 
+			if header.Size > maxExtractFileSize {
+				return fmt.Errorf("file %s exceeds maximum allowed size (%d bytes, limit %d)", name, header.Size, maxExtractFileSize)
+			}
+
 			f, err := os.OpenFile(targetPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(header.Mode)&0755|0600) //nolint:gosec // mode is masked to safe range
 			if err != nil {
 				return fmt.Errorf("failed to create file %s: %w", targetPath, err)
 			}
 
-			if _, err := io.Copy(f, tr); err != nil { //nolint:gosec // tar size is bounded by GitHub API tarball limits
+			written, err := io.CopyN(f, tr, maxExtractFileSize+1)
+			if err != nil && !errors.Is(err, io.EOF) {
 				f.Close()
 				return fmt.Errorf("failed to write file %s: %w", targetPath, err)
 			}
+			if written > maxExtractFileSize {
+				f.Close()
+				return fmt.Errorf("file %s exceeds maximum allowed size (limit %d bytes)", name, maxExtractFileSize)
+			}
 			f.Close()
+
+			totalSize += written
+			if totalSize > maxExtractTotalSize {
+				return fmt.Errorf("archive exceeds maximum total extracted size (limit %d bytes)", maxExtractTotalSize)
+			}
 		}
 	}
 

internal/templaterepo/client_test.go

@@ -1,6 +1,9 @@
 package templaterepo
 
 import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
@@ -14,6 +17,38 @@ import (
 	"github.com/smartcontractkit/cre-cli/internal/testutil"
 )
 
+// buildTarGz builds an in-memory gzip'd tarball with a synthetic top-level
+// prefix directory (mirroring GitHub's tarball layout) followed by the given
+// files, each written with the requested content.
+func buildTarGz(t *testing.T, files map[string]string) *bytes.Buffer {
+	t.Helper()
+
+	var buf bytes.Buffer
+	gw := gzip.NewWriter(&buf)
+	tw := tar.NewWriter(gw)
+
+	require.NoError(t, tw.WriteHeader(&tar.Header{
+		Name:     "owner-repo-abc123/",
+		Typeflag: tar.TypeDir,
+		Mode:     0755,
+	}))
+
+	for name, content := range files {
+		require.NoError(t, tw.WriteHeader(&tar.Header{
+			Name:     "owner-repo-abc123/" + name,
+			Typeflag: tar.TypeReg,
+			Mode:     0644,
+			Size:     int64(len(content)),
+		}))
+		_, err := tw.Write([]byte(content))
+		require.NoError(t, err)
+	}
+
+	require.NoError(t, tw.Close())
+	require.NoError(t, gw.Close())
+	return &buf
+}
+
 func TestDiscoverTemplates_FindsTemplateYaml(t *testing.T) {
 	logger := testutil.NewTestLogger()
 
@@ -143,3 +178,68 @@ func TestExtractTarball_BasicExtraction(t *testing.T) {
 
 	_ = client
 }
+
+func TestExtractTarball_EnforcesPerFileSizeLimit(t *testing.T) {
+	orig := maxExtractFileSize
+	maxExtractFileSize = 10
+	defer func() { maxExtractFileSize = orig }()
+
+	client := NewClient(testutil.NewTestLogger())
+	tarball := buildTarGz(t, map[string]string{
+		".cre/template.yaml": "kind: x\n",
+		"main.go":            "this file is way over the ten byte cap",
+	})
+
+	err := client.extractTarball(tarball, "", t.TempDir(), nil, false)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "exceeds maximum allowed size")
+}
+
+func TestExtractTarball_EnforcesTotalSizeLimit(t *testing.T) {
+	orig := maxExtractTotalSize
+	maxExtractTotalSize = 10
+	defer func() { maxExtractTotalSize = orig }()
+
+	client := NewClient(testutil.NewTestLogger())
+	tarball := buildTarGz(t, map[string]string{
+		"a.txt": "123456",
+		"b.txt": "789012",
+	})
+
+	err := client.extractTarball(tarball, "", t.TempDir(), nil, false)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "maximum total extracted size")
+}
+
+func TestExtractTarball_EnforcesFileCountLimit(t *testing.T) {
+	orig := maxExtractFileCount
+	maxExtractFileCount = 2
+	defer func() { maxExtractFileCount = orig }()
+
+	client := NewClient(testutil.NewTestLogger())
+	tarball := buildTarGz(t, map[string]string{
+		"a.txt": "1",
+		"b.txt": "2",
+		"c.txt": "3",
+	})
+
+	err := client.extractTarball(tarball, "", t.TempDir(), nil, false)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "too many entries")
+}
+
+func TestExtractTarball_RejectsPathTraversal(t *testing.T) {
+	client := NewClient(testutil.NewTestLogger())
+	destDir := t.TempDir()
+
+	tarball := buildTarGz(t, map[string]string{
+		"../../etc/passwd": "evil",
+	})
+
+	err := client.extractTarball(tarball, "", destDir, nil, false)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "illegal file path")
+
+	entries, _ := os.ReadDir(destDir)
+	assert.Empty(t, entries)
+}