Skip to content

Commit

Permalink
Merge pull request from GHSA-jhwx-mhww-rgc3
Browse files Browse the repository at this point in the history 
* sec: limit helm index max size

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* sec: limit helm index max size

Signed-off-by: pashakostohrys <pavel@codefresh.io>

* feat: fix tests and linter

Signed-off-by: pashakostohrys <pavel@codefresh.io>

---------

Signed-off-by: pashakostohrys <pavel@codefresh.io>
  • Loading branch information
@pasha-codefresh
pasha-codefresh committed Mar 28, 2024
1 parent 8281b18 commit 14f681e
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 14 deletions.
6 changes: 6 additions & 0 deletions cmd/argocd-repo-server/commands/argocd_repo_server.go
View file
Expand Up @@ -66,6 +66,7 @@ func NewCommand() *cobra.Command {
streamedManifestMaxTarSize string
streamedManifestMaxExtractedSize string
helmManifestMaxExtractedSize string
helmRegistryMaxIndexSize string
disableManifestMaxExtractedSize bool
)
var command = cobra.Command{
Expand Down Expand Up @@ -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)
Expand All @@ -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)

Expand Down Expand Up @@ -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) {
Expand Down
7 changes: 4 additions & 3 deletions reposerver/repository/repository.go
View file
Expand Up @@ -107,6 +107,7 @@ type RepoServerInitConstants struct {
StreamedManifestMaxExtractedSize int64
StreamedManifestMaxTarSize int64
HelmManifestMaxExtractedSize int64
HelmRegistryMaxIndexSize int64
DisableHelmManifestMaxExtractedSize bool
}

Expand Down Expand Up @@ -2345,7 +2346,7 @@ func (s *Service) newHelmClientResolveRevision(repo *v1alpha1.Repository, revisi
return helmClient, version.String(), nil
}

index, err := helmClient.GetIndex(noRevisionCache)
index, err := helmClient.GetIndex(noRevisionCache, s.initConstants.HelmRegistryMaxIndexSize)
if err != nil {
return nil, "", err
}
Expand Down Expand Up @@ -2423,7 +2424,7 @@ func checkoutRevision(gitClient git.Client, revision string, submoduleEnabled bo
}

func (s *Service) GetHelmCharts(ctx context.Context, q *apiclient.HelmChartsRequest) (*apiclient.HelmChartsResponse, error) {
index, err := s.newHelmClient(q.Repo.Repo, q.Repo.GetHelmCreds(), q.Repo.EnableOCI, q.Repo.Proxy, helm.WithChartPaths(s.chartPaths)).GetIndex(true)
index, err := s.newHelmClient(q.Repo.Repo, q.Repo.GetHelmCreds(), q.Repo.EnableOCI, q.Repo.Proxy, helm.WithChartPaths(s.chartPaths)).GetIndex(true, s.initConstants.HelmRegistryMaxIndexSize)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -2458,7 +2459,7 @@ func (s *Service) TestRepository(ctx context.Context, q *apiclient.TestRepositor
_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).TestHelmOCI()
return err
} else {
_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).GetIndex(false)
_, err := helm.NewClient(repo.Repo, repo.GetHelmCreds(), repo.EnableOCI, repo.Proxy).GetIndex(false, s.initConstants.HelmRegistryMaxIndexSize)
return err
}
},
Expand Down
2 changes: 1 addition & 1 deletion reposerver/repository/repository_test.go
View file
Expand Up @@ -113,7 +113,7 @@ func newServiceWithMocks(t *testing.T, root string, signed bool) (*Service, *git
chart := "my-chart"
oobChart := "out-of-bounds-chart"
version := "1.1.0"
helmClient.On("GetIndex", mock.AnythingOfType("bool")).Return(&helm.Index{Entries: map[string]helm.Entries{
helmClient.On("GetIndex", mock.AnythingOfType("bool"), mock.Anything).Return(&helm.Index{Entries: map[string]helm.Entries{
chart: {{Version: "1.0.0"}, {Version: version}},
oobChart: {{Version: "1.0.0"}, {Version: version}},
}}, nil)
Expand Down
10 changes: 5 additions & 5 deletions util/helm/client.go
View file
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)

Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
14 changes: 10 additions & 4 deletions util/helm/client_test.go
View file
Expand Up @@ -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)
})
Expand All @@ -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)
})
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion util/helm/mocks/Client.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 14f681e

Please sign in to comment.