From 477b37261165cba2c5153d44c922906de9f036ee Mon Sep 17 00:00:00 2001 From: pashakostohrys Date: Thu, 14 Mar 2024 20:11:23 +0200 Subject: [PATCH] sec: limit helm index max size Signed-off-by: pashakostohrys --- .../commands/argocd_repo_server.go | 6 ++++++ pkg/apis/application/v1alpha1/repository_types.go | 2 +- reposerver/repository/repository.go | 1 + util/helm/client.go | 10 +++++----- util/helm/client_test.go | 14 ++++++++++---- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/cmd/argocd-repo-server/commands/argocd_repo_server.go b/cmd/argocd-repo-server/commands/argocd_repo_server.go index 69358d2a91efd..2362e4496a4e8 100644 --- a/cmd/argocd-repo-server/commands/argocd_repo_server.go +++ b/cmd/argocd-repo-server/commands/argocd_repo_server.go @@ -66,6 +66,7 @@ func NewCommand() *cobra.Command { streamedManifestMaxTarSize string streamedManifestMaxExtractedSize string helmManifestMaxExtractedSize string + helmRegistryMaxIndexSize string disableManifestMaxExtractedSize bool ) var command = cobra.Command{ @@ -108,6 +109,9 @@ func NewCommand() *cobra.Command { helmManifestMaxExtractedSizeQuantity, err := resource.ParseQuantity(helmManifestMaxExtractedSize) errors.CheckError(err) + helmRegistryMaxIndexSizeQuantity, err := resource.ParseQuantity(helmRegistryMaxIndexSize) + errors.CheckError(err) + askPassServer := askpass.NewServer() metricsServer := metrics.NewMetricsServer() cacheutil.CollectMetrics(redisClient, metricsServer) @@ -123,6 +127,7 @@ func NewCommand() *cobra.Command { StreamedManifestMaxExtractedSize: streamedManifestMaxExtractedSizeQuantity.ToDec().Value(), StreamedManifestMaxTarSize: streamedManifestMaxTarSizeQuantity.ToDec().Value(), HelmManifestMaxExtractedSize: helmManifestMaxExtractedSizeQuantity.ToDec().Value(), + HelmRegistryMaxIndexSize: helmRegistryMaxIndexSizeQuantity.ToDec().Value(), }, askPassServer) errors.CheckError(err) @@ -204,6 +209,7 @@ func NewCommand() *cobra.Command { 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().StringVar(&helmRegistryMaxIndexSize, "helm-registry-max-index-size", env.StringFromEnv("ARGOCD_REPO_SERVER_HELM_MANIFEST_MAX_INDEX_SIZE", "1G"), "Maximum size of registry index file") 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) { diff --git a/pkg/apis/application/v1alpha1/repository_types.go b/pkg/apis/application/v1alpha1/repository_types.go index 31e8c47971414..27fd5a51784ec 100644 --- a/pkg/apis/application/v1alpha1/repository_types.go +++ b/pkg/apis/application/v1alpha1/repository_types.go @@ -236,7 +236,7 @@ func getCAPath(repoURL string) string { log.Warnf("Could not parse repo URL '%s': %v", repoURL, err) return "" } - if parsedURL.Scheme == "https" || parsedURL.Scheme == "oci" { + if parsedURL.Scheme == "https" || parsedURL.Scheme == "oci" || parsedURL.Scheme == "http" { hostname = parsedURL.Host } else if parsedURL.Scheme == "" { hostname = parsedURL.Path diff --git a/reposerver/repository/repository.go b/reposerver/repository/repository.go index 12e00fb855999..db7f5c423d222 100644 --- a/reposerver/repository/repository.go +++ b/reposerver/repository/repository.go @@ -107,6 +107,7 @@ type RepoServerInitConstants struct { StreamedManifestMaxExtractedSize int64 StreamedManifestMaxTarSize int64 HelmManifestMaxExtractedSize int64 + HelmRegistryMaxIndexSize int64 DisableHelmManifestMaxExtractedSize bool } diff --git a/util/helm/client.go b/util/helm/client.go index 75bd30d1fea13..8b99cd67c6904 100644 --- a/util/helm/client.go +++ b/util/helm/client.go @@ -56,7 +56,7 @@ type indexCache interface { type Client interface { CleanChartCache(chart string, version string) error ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, argoio.Closer, error) - GetIndex(noCache bool) (*Index, error) + GetIndex(noCache bool, maxIndexSize int64) (*Index, error) GetTags(chart string, noCache bool) (*TagsList, error) TestHelmOCI() (bool, error) } @@ -230,7 +230,7 @@ func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredent }), nil } -func (c *nativeHelmChart) GetIndex(noCache bool) (*Index, error) { +func (c *nativeHelmChart) GetIndex(noCache bool, maxIndexSize int64) (*Index, error) { indexLock.Lock(c.repoURL) defer indexLock.Unlock(c.repoURL) @@ -244,7 +244,7 @@ func (c *nativeHelmChart) GetIndex(noCache bool) (*Index, error) { if len(data) == 0 { start := time.Now() var err error - data, err = c.loadRepoIndex() + data, err = c.loadRepoIndex(maxIndexSize) if err != nil { return nil, err } @@ -297,7 +297,7 @@ func (c *nativeHelmChart) TestHelmOCI() (bool, error) { return true, nil } -func (c *nativeHelmChart) loadRepoIndex() ([]byte, error) { +func (c *nativeHelmChart) loadRepoIndex(maxIndexSize int64) ([]byte, error) { indexURL, err := getIndexURL(c.repoURL) if err != nil { return nil, err @@ -332,7 +332,7 @@ func (c *nativeHelmChart) loadRepoIndex() ([]byte, error) { if resp.StatusCode != http.StatusOK { return nil, errors.New("failed to get index: " + resp.Status) } - return io.ReadAll(resp.Body) + return io.ReadAll(io.LimitReader(resp.Body, maxIndexSize)) } func newTLSConfig(creds Creds) (*tls.Config, error) { diff --git a/util/helm/client_test.go b/util/helm/client_test.go index 6fba279df07d0..ad613ca3bd7eb 100644 --- a/util/helm/client_test.go +++ b/util/helm/client_test.go @@ -37,12 +37,12 @@ func (f *fakeIndexCache) GetHelmIndex(_ string, indexData *[]byte) error { func TestIndex(t *testing.T) { t.Run("Invalid", func(t *testing.T) { client := NewClient("", Creds{}, false, "") - _, err := client.GetIndex(false) + _, err := client.GetIndex(false, 10000) assert.Error(t, err) }) t.Run("Stable", func(t *testing.T) { client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "") - index, err := client.GetIndex(false) + index, err := client.GetIndex(false, 10000) assert.NoError(t, err) assert.NotNil(t, index) }) @@ -51,7 +51,7 @@ func TestIndex(t *testing.T) { Username: "my-password", Password: "my-username", }, false, "") - index, err := client.GetIndex(false) + index, err := client.GetIndex(false, 10000) assert.NoError(t, err) assert.NotNil(t, index) }) @@ -63,12 +63,18 @@ func TestIndex(t *testing.T) { require.NoError(t, err) client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "", WithIndexCache(&fakeIndexCache{data: data.Bytes()})) - index, err := client.GetIndex(false) + index, err := client.GetIndex(false, 10000) assert.NoError(t, err) assert.Equal(t, fakeIndex, *index) }) + t.Run("Limited", func(t *testing.T) { + client := NewClient("https://argoproj.github.io/argo-helm", Creds{}, false, "") + _, err := client.GetIndex(false, 100) + + assert.ErrorContains(t, err, "unexpected end of stream") + }) } func Test_nativeHelmChart_ExtractChart(t *testing.T) {