From 44e52c4ae76e6da1343bdd54e57a822d93549f28 Mon Sep 17 00:00:00 2001 From: pasha-codefresh Date: Thu, 7 Sep 2023 17:12:15 +0300 Subject: [PATCH] Merge pull request from GHSA-g687-f2gx-6wm8 * feat: use untar with limiter Signed-off-by: pashakostohrys * feat: use untar with limiter Signed-off-by: pashakostohrys --------- Signed-off-by: pashakostohrys --- .../commands/argocd_repo_server.go | 8 ++++++ .../server-commands/argocd-repo-server.md | 2 ++ .../argocd-repo-server-deployment.yaml | 12 +++++++++ manifests/core-install.yaml | 12 +++++++++ manifests/ha/install.yaml | 12 +++++++++ manifests/ha/namespace-install.yaml | 12 +++++++++ manifests/install.yaml | 12 +++++++++ manifests/namespace-install.yaml | 12 +++++++++ reposerver/repository/repository.go | 6 +++-- util/helm/client.go | 25 ++++++++++++++----- util/helm/client_test.go | 11 ++++++-- util/helm/mocks/Client.go | 2 +- 12 files changed, 115 insertions(+), 11 deletions(-) diff --git a/cmd/argocd-repo-server/commands/argocd_repo_server.go b/cmd/argocd-repo-server/commands/argocd_repo_server.go index b9a0378b13ed5..0d2fd16343a2e 100644 --- a/cmd/argocd-repo-server/commands/argocd_repo_server.go +++ b/cmd/argocd-repo-server/commands/argocd_repo_server.go @@ -82,6 +82,8 @@ func NewCommand() *cobra.Command { allowOutOfBoundsSymlinks bool streamedManifestMaxTarSize string streamedManifestMaxExtractedSize string + helmManifestMaxExtractedSize string + disableManifestMaxExtractedSize bool ) var command = cobra.Command{ Use: cliName, @@ -120,6 +122,9 @@ func NewCommand() *cobra.Command { streamedManifestMaxExtractedSizeQuantity, err := resource.ParseQuantity(streamedManifestMaxExtractedSize) errors.CheckError(err) + helmManifestMaxExtractedSizeQuantity, err := resource.ParseQuantity(helmManifestMaxExtractedSize) + errors.CheckError(err) + askPassServer := askpass.NewServer() metricsServer := metrics.NewMetricsServer() cacheutil.CollectMetrics(redisClient, metricsServer) @@ -134,6 +139,7 @@ func NewCommand() *cobra.Command { AllowOutOfBoundsSymlinks: allowOutOfBoundsSymlinks, StreamedManifestMaxExtractedSize: streamedManifestMaxExtractedSizeQuantity.ToDec().Value(), StreamedManifestMaxTarSize: streamedManifestMaxTarSizeQuantity.ToDec().Value(), + HelmManifestMaxExtractedSize: helmManifestMaxExtractedSizeQuantity.ToDec().Value(), }, askPassServer) errors.CheckError(err) @@ -216,6 +222,8 @@ func NewCommand() *cobra.Command { command.Flags().BoolVar(&allowOutOfBoundsSymlinks, "allow-oob-symlinks", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_ALLOW_OUT_OF_BOUNDS_SYMLINKS", false), "Allow out-of-bounds symlinks in repositories (not recommended)") command.Flags().StringVar(&streamedManifestMaxTarSize, "streamed-manifest-max-tar-size", env.StringFromEnv("ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_TAR_SIZE", "100M"), "Maximum size of streamed manifest archives") command.Flags().StringVar(&streamedManifestMaxExtractedSize, "streamed-manifest-max-extracted-size", env.StringFromEnv("ARGOCD_REPO_SERVER_STREAMED_MANIFEST_MAX_EXTRACTED_SIZE", "1G"), "Maximum size of streamed manifest archives when extracted") + command.Flags().StringVar(&helmManifestMaxExtractedSize, "helm-manifest-max-extracted-size", env.StringFromEnv("ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE", "1G"), "Maximum size of helm manifest archives when extracted") + command.Flags().BoolVar(&disableManifestMaxExtractedSize, "disable-helm-manifest-max-extracted-size", env.ParseBoolFromEnv("ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE", false), "Disable maximum size of helm manifest archives when extracted") tlsConfigCustomizerSrc = tls.AddTLSFlagsToCmd(&command) cacheSrc = reposervercache.AddCacheFlagsToCmd(&command, func(client *redis.Client) { redisClient = client diff --git a/docs/operator-manual/server-commands/argocd-repo-server.md b/docs/operator-manual/server-commands/argocd-repo-server.md index 35d8cbe8dfed5..805c6beab6436 100644 --- a/docs/operator-manual/server-commands/argocd-repo-server.md +++ b/docs/operator-manual/server-commands/argocd-repo-server.md @@ -16,7 +16,9 @@ argocd-repo-server [flags] --address string Listen on given address for incoming connections (default "0.0.0.0") --allow-oob-symlinks Allow out-of-bounds symlinks in repositories (not recommended) --default-cache-expiration duration Cache expiration default (default 24h0m0s) + --disable-helm-manifest-max-extracted-size Disable maximum size of helm manifest archives when extracted --disable-tls Disable TLS on the gRPC endpoint + --helm-manifest-max-extracted-size string Maximum size of helm manifest archives when extracted (default "1G") -h, --help help for argocd-repo-server --logformat string Set the logging format. One of: text|json (default "text") --loglevel string Set the logging level. One of: debug|info|warn|error (default "info") diff --git a/manifests/base/repo-server/argocd-repo-server-deployment.yaml b/manifests/base/repo-server/argocd-repo-server-deployment.yaml index 735f6436f6699..eb230e0f9b856 100644 --- a/manifests/base/repo-server/argocd-repo-server-deployment.yaml +++ b/manifests/base/repo-server/argocd-repo-server-deployment.yaml @@ -150,6 +150,18 @@ spec: key: reposerver.streamed.manifest.max.extracted.size name: argocd-cmd-params-cm optional: true + - name: ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true + - name: ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + name: argocd-cmd-params-cm + key: reposerver.disable.helm.manifest.max.extracted.size + optional: true - name: ARGOCD_GIT_MODULES_ENABLED valueFrom: configMapKeyRef: diff --git a/manifests/core-install.yaml b/manifests/core-install.yaml index fe150b316d1e8..e129c2bc99910 100644 --- a/manifests/core-install.yaml +++ b/manifests/core-install.yaml @@ -19156,6 +19156,18 @@ spec: key: reposerver.streamed.manifest.max.extracted.size name: argocd-cmd-params-cm optional: true + - name: ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true + - name: ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.disable.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_GIT_MODULES_ENABLED valueFrom: configMapKeyRef: diff --git a/manifests/ha/install.yaml b/manifests/ha/install.yaml index 1bfeb35e2c382..192def70b5014 100644 --- a/manifests/ha/install.yaml +++ b/manifests/ha/install.yaml @@ -20612,6 +20612,18 @@ spec: key: reposerver.streamed.manifest.max.extracted.size name: argocd-cmd-params-cm optional: true + - name: ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true + - name: ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.disable.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_GIT_MODULES_ENABLED valueFrom: configMapKeyRef: diff --git a/manifests/ha/namespace-install.yaml b/manifests/ha/namespace-install.yaml index eb10a4f0e93e7..02b1b034a00ce 100644 --- a/manifests/ha/namespace-install.yaml +++ b/manifests/ha/namespace-install.yaml @@ -2130,6 +2130,18 @@ spec: key: reposerver.streamed.manifest.max.extracted.size name: argocd-cmd-params-cm optional: true + - name: ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true + - name: ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.disable.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_GIT_MODULES_ENABLED valueFrom: configMapKeyRef: diff --git a/manifests/install.yaml b/manifests/install.yaml index ee7c6cc103ed1..ceb7cb3ce3f91 100644 --- a/manifests/install.yaml +++ b/manifests/install.yaml @@ -19669,6 +19669,18 @@ spec: key: reposerver.streamed.manifest.max.extracted.size name: argocd-cmd-params-cm optional: true + - name: ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true + - name: ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.disable.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_GIT_MODULES_ENABLED valueFrom: configMapKeyRef: diff --git a/manifests/namespace-install.yaml b/manifests/namespace-install.yaml index f32d3b2ed89b3..8309bf828efdf 100644 --- a/manifests/namespace-install.yaml +++ b/manifests/namespace-install.yaml @@ -1187,6 +1187,18 @@ spec: key: reposerver.streamed.manifest.max.extracted.size name: argocd-cmd-params-cm optional: true + - name: ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true + - name: ARGOCD_REPO_SERVER_DISABLE_HELM_MANIFEST_MAX_EXTRACTED_SIZE + valueFrom: + configMapKeyRef: + key: reposerver.disable.helm.manifest.max.extracted.size + name: argocd-cmd-params-cm + optional: true - name: ARGOCD_GIT_MODULES_ENABLED valueFrom: configMapKeyRef: diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 02fb6a20d49de..8fa4ddc7ad39e 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -107,6 +107,8 @@ type RepoServerInitConstants struct { AllowOutOfBoundsSymlinks bool StreamedManifestMaxExtractedSize int64 StreamedManifestMaxTarSize int64 + HelmManifestMaxExtractedSize int64 + DisableHelmManifestMaxExtractedSize bool } // NewService returns a new instance of the Manifest service @@ -346,7 +348,7 @@ func (s *Service) runRepoOperation( if source.Helm != nil { helmPassCredentials = source.Helm.PassCredentials } - chartPath, closer, err := helmClient.ExtractChart(source.Chart, revision, helmPassCredentials) + chartPath, closer, err := helmClient.ExtractChart(source.Chart, revision, helmPassCredentials, s.initConstants.HelmManifestMaxExtractedSize, s.initConstants.DisableHelmManifestMaxExtractedSize) if err != nil { return err } @@ -2233,7 +2235,7 @@ func (s *Service) GetRevisionChartDetails(ctx context.Context, q *apiclient.Repo if err != nil { return nil, fmt.Errorf("helm client error: %v", err) } - chartPath, closer, err := helmClient.ExtractChart(q.Name, revision, false) + chartPath, closer, err := helmClient.ExtractChart(q.Name, revision, false, s.initConstants.HelmManifestMaxExtractedSize, s.initConstants.DisableHelmManifestMaxExtractedSize) if err != nil { return nil, fmt.Errorf("error extracting chart: %v", err) } diff --git a/util/helm/client.go b/util/helm/client.go index 8096b8009dbb4..5e96e0535b4c9 100644 --- a/util/helm/client.go +++ b/util/helm/client.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + executil "github.com/argoproj/argo-cd/v2/util/exec" "io" "net/http" "net/url" @@ -25,7 +26,6 @@ import ( "oras.land/oras-go/v2/registry/remote/auth" "github.com/argoproj/argo-cd/v2/util/cache" - executil "github.com/argoproj/argo-cd/v2/util/exec" argoio "github.com/argoproj/argo-cd/v2/util/io" "github.com/argoproj/argo-cd/v2/util/io/files" "github.com/argoproj/argo-cd/v2/util/proxy" @@ -52,7 +52,7 @@ type indexCache interface { type Client interface { CleanChartCache(chart string, version string) error - ExtractChart(chart string, version string, passCredentials bool) (string, argoio.Closer, error) + ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) GetIndex(noCache bool) (*Index, error) GetTags(chart string, noCache bool) (*TagsList, error) TestHelmOCI() (bool, error) @@ -122,7 +122,21 @@ func (c *nativeHelmChart) CleanChartCache(chart string, version string) error { return os.RemoveAll(cachePath) } -func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredentials bool) (string, argoio.Closer, error) { +func untarChart(tempDir string, cachedChartPath string, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) error { + if disableManifestMaxExtractedSize { + cmd := exec.Command("tar", "-zxvf", cachedChartPath) + cmd.Dir = tempDir + _, err := executil.Run(cmd) + return err + } + reader, err := os.Open(cachedChartPath) + if err != nil { + return err + } + return files.Untgz(tempDir, reader, manifestMaxExtractedSize, false) +} + +func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) { // always use Helm V3 since we don't have chart content to determine correct Helm version helmCmd, err := NewCmdWithVersion("", HelmV3, c.enableOci, c.proxy) @@ -196,15 +210,14 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredent if len(infos) != 1 { return "", nil, fmt.Errorf("expected 1 file, found %v", len(infos)) } + err = os.Rename(filepath.Join(tempDest, infos[0].Name()), cachedChartPath) if err != nil { return "", nil, err } } - cmd := exec.Command("tar", "-zxvf", cachedChartPath) - cmd.Dir = tempDir - _, err = executil.Run(cmd) + err = untarChart(tempDir, cachedChartPath, manifestMaxExtractedSize, disableManifestMaxExtractedSize) if err != nil { _ = os.RemoveAll(tempDir) return "", nil, err diff --git a/util/helm/client_test.go b/util/helm/client_test.go index 63b57b7fa5180..3cda26feb5f0e 100644 --- a/util/helm/client_test.go +++ b/util/helm/client_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "fmt" + "math" "os" "strings" "testing" @@ -71,7 +72,7 @@ func TestIndex(t *testing.T) { func Test_nativeHelmChart_ExtractChart(t *testing.T) { client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "") - path, closer, err := client.ExtractChart("argo-cd", "0.7.1", false) + path, closer, err := client.ExtractChart("argo-cd", "0.7.1", false, math.MaxInt64, true) assert.NoError(t, err) defer io.Close(closer) info, err := os.Stat(path) @@ -79,9 +80,15 @@ func Test_nativeHelmChart_ExtractChart(t *testing.T) { assert.True(t, info.IsDir()) } +func Test_nativeHelmChart_ExtractChartWithLimiter(t *testing.T) { + client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "") + _, _, err := client.ExtractChart("argo-cd", "0.7.1", false, 100, false) + assert.Error(t, err, "error while iterating on tar reader: unexpected EOF") +} + func Test_nativeHelmChart_ExtractChart_insecure(t *testing.T) { client := NewClient("https://argoproj.github.io/argo-helm", Creds{InsecureSkipVerify: true}, false, "") - path, closer, err := client.ExtractChart("argo-cd", "0.7.1", false) + path, closer, err := client.ExtractChart("argo-cd", "0.7.1", false, math.MaxInt64, true) assert.NoError(t, err) defer io.Close(closer) info, err := os.Stat(path) diff --git a/util/helm/mocks/Client.go b/util/helm/mocks/Client.go index 31b5a11e25110..6dc25e4affd0b 100644 --- a/util/helm/mocks/Client.go +++ b/util/helm/mocks/Client.go @@ -29,7 +29,7 @@ func (_m *Client) CleanChartCache(chart string, version string) error { } // ExtractChart provides a mock function with given fields: chart, version -func (_m *Client) ExtractChart(chart string, version string, passCredentials bool) (string, io.Closer, error) { +func (_m *Client) ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, io.Closer, error) { ret := _m.Called(chart, version) var r0 string