artifact: protect against unbounded artifact decompression (1.5.0) (#…

…16151)

* artifact: protect against unbounded artifact decompression

Starting with 1.5.0, set defaut values for artifact decompression limits.

artifact.decompression_size_limit (default "100GB") - the maximum amount of
data that will be decompressed before triggering an error and cancelling
the operation

artifact.decompression_file_count_limit (default 4096) - the maximum number
of files that will be decompressed before triggering an error and
cancelling the operation.

* artifact: assert limits cannot be nil in validation

.changelog/16151.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+artifact: Provide mitigations against unbounded artifact decompression
+```

client/allocrunner/taskrunner/getter/params.go

@@ -21,14 +21,16 @@ import (
 // e.g. https://www.opencve.io/cve/CVE-2022-41716
 type parameters struct {
 	// Config
-	HTTPReadTimeout            time.Duration `json:"http_read_timeout"`
-	HTTPMaxBytes               int64         `json:"http_max_bytes"`
-	GCSTimeout                 time.Duration `json:"gcs_timeout"`
-	GitTimeout                 time.Duration `json:"git_timeout"`
-	HgTimeout                  time.Duration `json:"hg_timeout"`
-	S3Timeout                  time.Duration `json:"s3_timeout"`
-	DisableFilesystemIsolation bool          `json:"disable_filesystem_isolation"`
-	SetEnvironmentVariables    string        `json:"set_environment_variables"`
+	HTTPReadTimeout             time.Duration `json:"http_read_timeout"`
+	HTTPMaxBytes                int64         `json:"http_max_bytes"`
+	GCSTimeout                  time.Duration `json:"gcs_timeout"`
+	GitTimeout                  time.Duration `json:"git_timeout"`
+	HgTimeout                   time.Duration `json:"hg_timeout"`
+	S3Timeout                   time.Duration `json:"s3_timeout"`
+	DecompressionLimitFileCount int           `json:"decompression_limit_file_count"`
+	DecompressionLimitSize      int64         `json:"decompression_limit_size"`
+	DisableFilesystemIsolation  bool          `json:"disable_filesystem_isolation"`
+	SetEnvironmentVariables     string        `json:"set_environment_variables"`
 
 	// Artifact
 	Mode        getter.ClientMode   `json:"artifact_mode"`
@@ -88,6 +90,10 @@ func (p *parameters) Equal(o *parameters) bool {
 		return false
 	case p.S3Timeout != o.S3Timeout:
 		return false
+	case p.DecompressionLimitFileCount != o.DecompressionLimitFileCount:
+		return false
+	case p.DecompressionLimitSize != o.DecompressionLimitSize:
+		return false
 	case p.DisableFilesystemIsolation != o.DisableFilesystemIsolation:
 		return false
 	case p.SetEnvironmentVariables != o.SetEnvironmentVariables:
@@ -136,6 +142,13 @@ func (p *parameters) client(ctx context.Context) *getter.Client {
 		// large downloads.
 		MaxBytes: p.HTTPMaxBytes,
 	}
+
+	// setup custom decompressors with file count and total size limits
+	decompressors := getter.LimitedDecompressors(
+		p.DecompressionLimitFileCount,
+		p.DecompressionLimitSize,
+	)
+
 	return &getter.Client{
 		Ctx:             ctx,
 		Src:             p.Source,
@@ -144,6 +157,7 @@ func (p *parameters) client(ctx context.Context) *getter.Client {
 		Umask:           umask,
 		Insecure:        false,
 		DisableSymlinks: true,
+		Decompressors:   decompressors,
 		Getters: map[string]getter.Getter{
 			"git": &getter.GitGetter{
 				Timeout: p.GitTimeout,

client/allocrunner/taskrunner/getter/params_test.go

@@ -19,6 +19,8 @@ const paramsAsJSON = `
   "git_timeout": 3000000000,
   "hg_timeout": 4000000000,
   "s3_timeout": 5000000000,
+  "decompression_limit_file_count": 3,
+  "decompression_limit_size": 98765,
   "disable_filesystem_isolation": true,
   "set_environment_variables": "",
   "artifact_mode": 2,
@@ -32,13 +34,15 @@ const paramsAsJSON = `
 }`
 
 var paramsAsStruct = &parameters{
-	HTTPReadTimeout:            1 * time.Second,
-	HTTPMaxBytes:               2000,
-	GCSTimeout:                 2 * time.Second,
-	GitTimeout:                 3 * time.Second,
-	HgTimeout:                  4 * time.Second,
-	S3Timeout:                  5 * time.Second,
-	DisableFilesystemIsolation: true,
+	HTTPReadTimeout:             1 * time.Second,
+	HTTPMaxBytes:                2000,
+	GCSTimeout:                  2 * time.Second,
+	GitTimeout:                  3 * time.Second,
+	HgTimeout:                   4 * time.Second,
+	S3Timeout:                   5 * time.Second,
+	DecompressionLimitFileCount: 3,
+	DecompressionLimitSize:      98765,
+	DisableFilesystemIsolation:  true,
 
 	Mode:        getter.ClientModeFile,
 	Source:      "https://example.com/file.txt",
@@ -98,6 +102,16 @@ func TestParameters_client(t *testing.T) {
 	// artifact options
 	must.Eq(t, "https://example.com/file.txt", c.Src)
 	must.Eq(t, "local/out.txt", c.Dst)
+
+	// decompression limits
+	const fileCountLimit = 3
+	const fileSizeLimit = 98765
+	must.Eq(t, fileSizeLimit, c.Decompressors["zip"].(*getter.ZipDecompressor).FileSizeLimit)
+	must.Eq(t, fileCountLimit, c.Decompressors["zip"].(*getter.ZipDecompressor).FilesLimit)
+	must.Eq(t, fileSizeLimit, c.Decompressors["tar.gz"].(*getter.TarGzipDecompressor).FileSizeLimit)
+	must.Eq(t, fileCountLimit, c.Decompressors["tar.gz"].(*getter.TarGzipDecompressor).FilesLimit)
+	must.Eq(t, fileSizeLimit, c.Decompressors["xz"].(*getter.XzDecompressor).FileSizeLimit)
+	// xz does not support files count limit
 }
 
 func TestParameters_Equal_headers(t *testing.T) {

client/allocrunner/taskrunner/getter/sandbox.go

@@ -40,14 +40,16 @@ func (s *Sandbox) Get(env interfaces.EnvReplacer, artifact *structs.TaskArtifact
 
 	params := &parameters{
 		// downloader configuration
-		HTTPReadTimeout:            s.ac.HTTPReadTimeout,
-		HTTPMaxBytes:               s.ac.HTTPMaxBytes,
-		GCSTimeout:                 s.ac.GCSTimeout,
-		GitTimeout:                 s.ac.GitTimeout,
-		HgTimeout:                  s.ac.HgTimeout,
-		S3Timeout:                  s.ac.S3Timeout,
-		DisableFilesystemIsolation: s.ac.DisableFilesystemIsolation,
-		SetEnvironmentVariables:    s.ac.SetEnvironmentVariables,
+		HTTPReadTimeout:             s.ac.HTTPReadTimeout,
+		HTTPMaxBytes:                s.ac.HTTPMaxBytes,
+		GCSTimeout:                  s.ac.GCSTimeout,
+		GitTimeout:                  s.ac.GitTimeout,
+		HgTimeout:                   s.ac.HgTimeout,
+		S3Timeout:                   s.ac.S3Timeout,
+		DecompressionLimitFileCount: s.ac.DecompressionLimitFileCount,
+		DecompressionLimitSize:      s.ac.DecompressionLimitSize,
+		DisableFilesystemIsolation:  s.ac.DisableFilesystemIsolation,
+		SetEnvironmentVariables:     s.ac.SetEnvironmentVariables,
 
 		// artifact configuration
 		Mode:        mode,

client/allocrunner/taskrunner/getter/testing.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	cconfig "github.com/hashicorp/nomad/client/config"
+	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/helper/testlog"
 	sconfig "github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/shoenig/test/must"
@@ -14,7 +15,10 @@ import (
 // TestSandbox creates a real artifact downloader configured via the default
 // artifact config. It is good enough for tests so no mock implementation exists.
 func TestSandbox(t *testing.T) *Sandbox {
-	ac, err := cconfig.ArtifactConfigFromAgent(sconfig.DefaultArtifactConfig())
+	defaultConfig := sconfig.DefaultArtifactConfig()
+	defaultConfig.DecompressionSizeLimit = pointer.Of("1MB")
+	defaultConfig.DecompressionFileCountLimit = pointer.Of(10)
+	ac, err := cconfig.ArtifactConfigFromAgent(defaultConfig)
 	must.NoError(t, err)
 	return New(ac, testlog.HCLogger(t))
 }

client/config/artifact.go

@@ -19,6 +19,9 @@ type ArtifactConfig struct {
 	HgTimeout  time.Duration
 	S3Timeout  time.Duration
 
+	DecompressionLimitFileCount int
+	DecompressionLimitSize      int64
+
 	DisableFilesystemIsolation bool
 	SetEnvironmentVariables    string
 }
@@ -56,15 +59,22 @@ func ArtifactConfigFromAgent(c *config.ArtifactConfig) (*ArtifactConfig, error)
 		return nil, fmt.Errorf("error parsing S3Timeout: %w", err)
 	}
 
+	decompressionSizeLimit, err := humanize.ParseBytes(*c.DecompressionSizeLimit)
+	if err != nil {
+		return nil, fmt.Errorf("error parsing DecompressionLimitSize: %w", err)
+	}
+
 	return &ArtifactConfig{
-		HTTPReadTimeout:            httpReadTimeout,
-		HTTPMaxBytes:               int64(httpMaxSize),
-		GCSTimeout:                 gcsTimeout,
-		GitTimeout:                 gitTimeout,
-		HgTimeout:                  hgTimeout,
-		S3Timeout:                  s3Timeout,
-		DisableFilesystemIsolation: *c.DisableFilesystemIsolation,
-		SetEnvironmentVariables:    *c.SetEnvironmentVariables,
+		HTTPReadTimeout:             httpReadTimeout,
+		HTTPMaxBytes:                int64(httpMaxSize),
+		GCSTimeout:                  gcsTimeout,
+		GitTimeout:                  gitTimeout,
+		HgTimeout:                   hgTimeout,
+		S3Timeout:                   s3Timeout,
+		DecompressionLimitFileCount: *c.DecompressionFileCountLimit,
+		DecompressionLimitSize:      int64(decompressionSizeLimit),
+		DisableFilesystemIsolation:  *c.DisableFilesystemIsolation,
+		SetEnvironmentVariables:     *c.SetEnvironmentVariables,
 	}, nil
 }
 

client/config/artifact_test.go

@@ -23,12 +23,14 @@ func TestArtifactConfigFromAgent(t *testing.T) {
 			name:   "from default",
 			config: config.DefaultArtifactConfig(),
 			exp: &ArtifactConfig{
-				HTTPReadTimeout: 30 * time.Minute,
-				HTTPMaxBytes:    100_000_000_000,
-				GCSTimeout:      30 * time.Minute,
-				GitTimeout:      30 * time.Minute,
-				HgTimeout:       30 * time.Minute,
-				S3Timeout:       30 * time.Minute,
+				HTTPReadTimeout:             30 * time.Minute,
+				HTTPMaxBytes:                100_000_000_000,
+				GCSTimeout:                  30 * time.Minute,
+				GitTimeout:                  30 * time.Minute,
+				HgTimeout:                   30 * time.Minute,
+				S3Timeout:                   30 * time.Minute,
+				DecompressionLimitFileCount: 4096,
+				DecompressionLimitSize:      100_000_000_000,
 			},
 		},
 		{

e2e/artifact/artifact_test.go

@@ -17,6 +17,7 @@ func TestArtifact(t *testing.T) {
 
 	t.Run("testLinux", testLinux)
 	t.Run("testWindows", testWindows)
+	t.Run("testLimits", testLimits)
 }
 
 // artifactCheckLogContents verifies expected logs for artifact downloader tests.
@@ -43,8 +44,7 @@ func artifactCheckLogContents(t *testing.T, nomad *api.Client, group, task strin
 
 func testWindows(t *testing.T) {
 	nomad := e2eutil.NomadClient(t)
-
-	jobID := "artifact-linux-" + uuid.Short()
+	jobID := "artifact-windows-" + uuid.Short()
 	jobIDs := []string{jobID}
 	t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs))
 
@@ -73,7 +73,6 @@ func testWindows(t *testing.T) {
 
 func testLinux(t *testing.T) {
 	nomad := e2eutil.NomadClient(t)
-
 	jobID := "artifact-linux-" + uuid.Short()
 	jobIDs := []string{jobID}
 	t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs))
@@ -113,3 +112,35 @@ func testLinux(t *testing.T) {
 	check("docker", "docker_zip_custom")
 	check("docker", "docker_git_custom")
 }
+
+func testLimits(t *testing.T) {
+	// defaults are 100GB, 4096 files; we run into the files count here
+
+	jobID := "artifact-limits-" + uuid.Short()
+	jobIDs := []string{jobID}
+	t.Cleanup(e2eutil.CleanupJobsAndGC(t, &jobIDs))
+
+	err := e2eutil.Register(jobID, "./input/artifact_limits.nomad")
+	must.NoError(t, err)
+
+	err = e2eutil.WaitForAllocStatusExpected(jobID, "", []string{"failed"})
+	must.NoError(t, err)
+
+	m, err := e2eutil.AllocTaskEventsForJob(jobID, "")
+	must.NoError(t, err)
+
+	found := false
+SCAN:
+	for _, events := range m {
+		for _, event := range events {
+			for label, description := range event {
+				if label == "Type" && description == "Failed Artifact Download" {
+					found = true
+					break SCAN
+				}
+
+			}
+		}
+	}
+	must.True(t, found)
+}

e2e/artifact/input/artifact_limits.nomad

@@ -0,0 +1,40 @@
+job "linux" {
+  datacenters = ["dc1"]
+  type        = "batch"
+
+  constraint {
+    attribute = "${attr.kernel.name}"
+    value     = "linux"
+  }
+
+  group "limits" {
+
+    reschedule {
+      attempts  = 0
+      unlimited = false
+    }
+
+    restart {
+      attempts = 0
+      mode     = "fail"
+    }
+
+
+    task "zip_bomb" {
+      artifact {
+        source      = "https://github.com/hashicorp/go-getter/raw/main/testdata/decompress-zip/bomb.zip"
+        destination = "local/"
+      }
+
+      driver = "raw_exec"
+      config {
+        command = "/usr/bin/false"
+      }
+
+      resources {
+        cpu    = 16
+        memory = 32
+      }
+    }
+  }
+}

go.mod

@@ -54,7 +54,7 @@ require (
 	// versions.
 	github.com/hashicorp/go-discover v0.0.0-20220621183603-a413e131e836
 	github.com/hashicorp/go-envparse v0.0.0-20180119215841-310ca1881b22
-	github.com/hashicorp/go-getter v1.6.2
+	github.com/hashicorp/go-getter v1.7.0
 	github.com/hashicorp/go-hclog v1.4.0
 	github.com/hashicorp/go-immutable-radix/v2 v2.0.0
 	github.com/hashicorp/go-kms-wrapping/v2 v2.0.5
@@ -224,7 +224,7 @@ require (
 	github.com/jefferai/isbadcipher v0.0.0-20190226160619-51d2077c035f // indirect
 	github.com/jmespath/go-jmespath v0.4.0 // indirect
 	github.com/joyent/triton-go v0.0.0-20190112182421-51ffac552869 // indirect
-	github.com/klauspost/compress v1.13.6 // indirect
+	github.com/klauspost/compress v1.15.11 // indirect
 	github.com/linode/linodego v0.7.1 // indirect
 	github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect
 	github.com/mattn/go-isatty v0.0.16 // indirect
@@ -272,7 +272,6 @@ require (
 	github.com/yusufpapurcu/wmi v1.2.2 // indirect
 	go.opencensus.io v0.24.0 // indirect
 	go.uber.org/atomic v1.9.0 // indirect
-	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect
 	golang.org/x/mod v0.6.0 // indirect
 	golang.org/x/net v0.5.0 // indirect
 	golang.org/x/oauth2 v0.4.0 // indirect