diff --git a/internal/packageimport/importer.go b/internal/packageimport/importer.go index c685e7b5..b69dc4fe 100644 --- a/internal/packageimport/importer.go +++ b/internal/packageimport/importer.go @@ -29,6 +29,20 @@ type Importer struct { Store *store.Store } +const maxRemoteArchiveBytes int64 = 512 << 20 + +var defaultArchiveExtractionLimits = archiveExtractionLimits{ + MaxFiles: 100_000, + MaxEntryBytes: 512 << 20, + MaxTotalBytes: 2 << 30, +} + +type archiveExtractionLimits struct { + MaxFiles int + MaxEntryBytes uint64 + MaxTotalBytes uint64 +} + type Options struct { ServiceID string `json:"service_id"` Name string `json:"name"` @@ -649,7 +663,7 @@ func (i *Importer) prepareSource(ctx context.Context, opts Options, staging stri err = untarGz(artifactPath, packageDir) } if err != nil { - return preparedSource{}, err + return preparedSource{}, fmt.Errorf("extract remote package %q: %w", redactedRemoteArchiveSource(source), err) } packageDir = normalizePackageDir(packageDir) } @@ -832,6 +846,9 @@ func downloadRemoteArchive(ctx context.Context, source, artifactPath string) err if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("download remote package %q: HTTP %d", redactedRemoteArchiveSource(source), resp.StatusCode) } + if resp.ContentLength > maxRemoteArchiveBytes { + return fmt.Errorf("download remote package %q: archive exceeds %d byte limit", redactedRemoteArchiveSource(source), maxRemoteArchiveBytes) + } if err := os.MkdirAll(filepath.Dir(artifactPath), 0o755); err != nil { return err } @@ -839,14 +856,29 @@ func downloadRemoteArchive(ctx context.Context, source, artifactPath string) err if err != nil { return err } - _, copyErr := io.Copy(out, resp.Body) + copyErr := copyWithByteLimit(out, resp.Body, maxRemoteArchiveBytes) closeErr := out.Close() if copyErr != nil { - return copyErr + _ = os.Remove(artifactPath) + return fmt.Errorf("download remote package %q: %w", redactedRemoteArchiveSource(source), copyErr) + } + if closeErr != nil { + _ = os.Remove(artifactPath) } return closeErr } +func copyWithByteLimit(dst io.Writer, src io.Reader, max int64) error { + written, err := io.Copy(dst, io.LimitReader(src, max+1)) + if err != nil { + return err + } + if written > max { + return fmt.Errorf("content exceeds %d byte limit", max) + } + return nil +} + func redactedRemoteArchiveSource(source string) string { u, err := url.Parse(source) if err != nil { @@ -1630,6 +1662,10 @@ func tarGzDir(src, dst string) error { } func untarGz(src, dst string) error { + return untarGzWithLimits(src, dst, defaultArchiveExtractionLimits) +} + +func untarGzWithLimits(src, dst string, limits archiveExtractionLimits) error { file, err := os.Open(src) if err != nil { return err @@ -1641,6 +1677,8 @@ func untarGz(src, dst string) error { } defer gz.Close() tr := tar.NewReader(gz) + files := 0 + var total uint64 for { hdr, err := tr.Next() if errors.Is(err, io.EOF) { @@ -1649,6 +1687,17 @@ func untarGz(src, dst string) error { if err != nil { return err } + files++ + if files > limits.MaxFiles { + return fmt.Errorf("archive exceeds %d file limit", limits.MaxFiles) + } + if hdr.Size < 0 || uint64(hdr.Size) > limits.MaxEntryBytes { + return fmt.Errorf("archive entry %q exceeds %d byte limit", hdr.Name, limits.MaxEntryBytes) + } + if uint64(hdr.Size) > limits.MaxTotalBytes-total { + return fmt.Errorf("archive exceeds %d expanded byte limit", limits.MaxTotalBytes) + } + total += uint64(hdr.Size) clean := filepath.Clean(hdr.Name) if strings.HasPrefix(clean, "..") || filepath.IsAbs(clean) { return fmt.Errorf("unsafe archive path %q", hdr.Name) @@ -1667,7 +1716,7 @@ func untarGz(src, dst string) error { if err != nil { return err } - _, copyErr := io.Copy(out, tr) + copyErr := copyWithByteLimit(out, tr, hdr.Size) closeErr := out.Close() if copyErr != nil { return copyErr @@ -1680,12 +1729,27 @@ func untarGz(src, dst string) error { } func unzip(src, dst string) error { + return unzipWithLimits(src, dst, defaultArchiveExtractionLimits) +} + +func unzipWithLimits(src, dst string, limits archiveExtractionLimits) error { r, err := zip.OpenReader(src) if err != nil { return err } defer r.Close() + if len(r.File) > limits.MaxFiles { + return fmt.Errorf("archive exceeds %d file limit", limits.MaxFiles) + } + var total uint64 for _, file := range r.File { + if file.UncompressedSize64 > limits.MaxEntryBytes { + return fmt.Errorf("archive entry %q exceeds %d byte limit", file.Name, limits.MaxEntryBytes) + } + if file.UncompressedSize64 > limits.MaxTotalBytes-total { + return fmt.Errorf("archive exceeds %d expanded byte limit", limits.MaxTotalBytes) + } + total += file.UncompressedSize64 clean := filepath.Clean(file.Name) if strings.HasPrefix(clean, "..") || filepath.IsAbs(clean) { return fmt.Errorf("unsafe archive path %q", file.Name) @@ -1709,7 +1773,7 @@ func unzip(src, dst string) error { _ = in.Close() return err } - _, copyErr := io.Copy(out, in) + copyErr := copyWithByteLimit(out, in, int64(file.UncompressedSize64)) closeIn := in.Close() closeOut := out.Close() if copyErr != nil { diff --git a/internal/packageimport/importer_test.go b/internal/packageimport/importer_test.go index a6391d6d..7e813cbe 100644 --- a/internal/packageimport/importer_test.go +++ b/internal/packageimport/importer_test.go @@ -1931,7 +1931,7 @@ func TestTarGzUntarAndUnzipHelpers(t *testing.T) { t.Fatalf("expected unsafe tgz path error, got %v", err) } unsafeZip := filepath.Join(dir, "unsafe.zip") - writeZipArchive(t, unsafeZip, "../escape.txt", "bad") + writeZipArchive(t, unsafeZip, zipEntry{name: "../escape.txt", body: "bad"}) if err := unzip(unsafeZip, filepath.Join(dir, "unsafe-zip-out")); err == nil || !strings.Contains(err.Error(), "unsafe archive path") { t.Fatalf("expected unsafe zip path error, got %v", err) } @@ -2409,7 +2409,12 @@ func writeTarArchive(t *testing.T, dst string, entries ...tarEntry) { } } -func writeZipArchive(t *testing.T, dst, name, body string) { +type zipEntry struct { + name string + body string +} + +func writeZipArchive(t *testing.T, dst string, entries ...zipEntry) { t.Helper() out, err := os.Create(dst) if err != nil { @@ -2418,12 +2423,14 @@ func writeZipArchive(t *testing.T, dst, name, body string) { defer out.Close() zw := zip.NewWriter(out) defer zw.Close() - w, err := zw.Create(name) - if err != nil { - t.Fatal(err) - } - if _, err := w.Write([]byte(body)); err != nil { - t.Fatal(err) + for _, entry := range entries { + w, err := zw.Create(entry.name) + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte(entry.body)); err != nil { + t.Fatal(err) + } } } diff --git a/internal/packageimport/resource_limits_test.go b/internal/packageimport/resource_limits_test.go new file mode 100644 index 00000000..e54ce166 --- /dev/null +++ b/internal/packageimport/resource_limits_test.go @@ -0,0 +1,150 @@ +package packageimport + +import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestCopyWithByteLimitAllowsExactSize(t *testing.T) { + var dst bytes.Buffer + if err := copyWithByteLimit(&dst, strings.NewReader("exact"), int64(len("exact"))); err != nil { + t.Fatal(err) + } + if dst.String() != "exact" { + t.Fatalf("copied content = %q", dst.String()) + } +} + +func TestCopyWithByteLimitRejectsOversizedContent(t *testing.T) { + var dst bytes.Buffer + err := copyWithByteLimit(&dst, strings.NewReader("oversized"), 4) + if err == nil || !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("copy limit error = %v", err) + } +} + +func TestTarExtractionEnforcesExpandedSizeLimit(t *testing.T) { + archivePath := filepath.Join(t.TempDir(), "package.tgz") + file, err := os.Create(archivePath) + if err != nil { + t.Fatal(err) + } + gz := gzip.NewWriter(file) + tarWriter := tar.NewWriter(gz) + body := []byte("0123456789") + if err := tarWriter.WriteHeader(&tar.Header{Name: "package/data", Mode: 0o600, Size: int64(len(body))}); err != nil { + t.Fatal(err) + } + if _, err := tarWriter.Write(body); err != nil { + t.Fatal(err) + } + if err := tarWriter.Close(); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + + err = untarGzWithLimits(archivePath, t.TempDir(), archiveExtractionLimits{ + MaxFiles: 10, + MaxEntryBytes: 5, + MaxTotalBytes: 5, + }) + if err == nil || !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("tar limit error = %v", err) + } +} + +func TestTarExtractionEnforcesFileCountLimit(t *testing.T) { + archivePath := filepath.Join(t.TempDir(), "package.tgz") + writeTarArchive(t, archivePath, tarEntry{name: "package/a", body: "a"}, tarEntry{name: "package/b", body: "b"}) + err := untarGzWithLimits(archivePath, t.TempDir(), archiveExtractionLimits{MaxFiles: 1, MaxEntryBytes: 10, MaxTotalBytes: 10}) + if err == nil || !strings.Contains(err.Error(), "file limit") { + t.Fatalf("tar file limit error = %v", err) + } +} + +func TestTarExtractionEnforcesTotalExpandedSizeLimit(t *testing.T) { + archivePath := filepath.Join(t.TempDir(), "package.tgz") + writeTarArchive(t, archivePath, tarEntry{name: "package/a", body: "abc"}, tarEntry{name: "package/b", body: "def"}) + err := untarGzWithLimits(archivePath, t.TempDir(), archiveExtractionLimits{MaxFiles: 10, MaxEntryBytes: 10, MaxTotalBytes: 5}) + if err == nil || !strings.Contains(err.Error(), "expanded byte limit") { + t.Fatalf("tar total size error = %v", err) + } +} + +func TestZipExtractionEnforcesEntryAndTotalSizeLimits(t *testing.T) { + // Entry-size check: a single oversized entry exceeds MaxEntryBytes. + oversizedEntry := filepath.Join(t.TempDir(), "oversized-entry.zip") + writeZipArchive(t, oversizedEntry, zipEntry{name: "package/a", body: "abcdef"}) + if err := unzipWithLimits(oversizedEntry, t.TempDir(), archiveExtractionLimits{MaxFiles: 10, MaxEntryBytes: 2, MaxTotalBytes: 10}); err == nil || !strings.Contains(err.Error(), "byte limit") { + t.Fatalf("zip entry size error = %v", err) + } + + // Total-size check: two 3-byte entries each stay under MaxEntryBytes=10 + // but total 6 bytes, which must trip the cumulative expanded-size check + // against MaxTotalBytes=5. + archivePath := filepath.Join(t.TempDir(), "package.zip") + writeZipArchive(t, archivePath, zipEntry{name: "package/a", body: "abc"}, zipEntry{name: "package/b", body: "def"}) + if err := unzipWithLimits(archivePath, t.TempDir(), archiveExtractionLimits{MaxFiles: 10, MaxEntryBytes: 10, MaxTotalBytes: 5}); err == nil || !strings.Contains(err.Error(), "expanded byte limit") { + t.Fatalf("zip total size error = %v", err) + } +} + +func TestZipExtractionAllowsArchiveWithinLimits(t *testing.T) { + archivePath := filepath.Join(t.TempDir(), "package.zip") + writeZipArchive(t, archivePath, zipEntry{name: "package/data", body: "ok"}) + dst := t.TempDir() + if err := unzipWithLimits(archivePath, dst, archiveExtractionLimits{MaxFiles: 10, MaxEntryBytes: 10, MaxTotalBytes: 10}); err != nil { + t.Fatal(err) + } + content, err := os.ReadFile(filepath.Join(dst, "package", "data")) + if err != nil { + t.Fatal(err) + } + if string(content) != "ok" { + t.Fatalf("extracted content = %q", content) + } +} + +func TestZipExtractionEnforcesFileCountLimit(t *testing.T) { + archivePath := filepath.Join(t.TempDir(), "package.zip") + file, err := os.Create(archivePath) + if err != nil { + t.Fatal(err) + } + zipWriter := zip.NewWriter(file) + for _, name := range []string{"package/a", "package/b"} { + entry, err := zipWriter.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := entry.Write(bytes.Repeat([]byte{'x'}, 1)); err != nil { + t.Fatal(err) + } + } + if err := zipWriter.Close(); err != nil { + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } + + err = unzipWithLimits(archivePath, t.TempDir(), archiveExtractionLimits{ + MaxFiles: 1, + MaxEntryBytes: 10, + MaxTotalBytes: 10, + }) + if err == nil || !strings.Contains(err.Error(), "file limit") { + t.Fatalf("zip limit error = %v", err) + } +}