VYPR
High severityNVD Advisory· Published Jul 22, 2024· Updated Aug 2, 2024

Argo CD Unauthenticated Denial of Service (DoS) Vulnerability via /api/webhook Endpoint

CVE-2024-40634

Description

Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes. This report details a security vulnerability in Argo CD, where an unauthenticated attacker can send a specially crafted large JSON payload to the /api/webhook endpoint, causing excessive memory allocation that leads to service disruption by triggering an Out Of Memory (OOM) kill. The issue poses a high risk to the availability of Argo CD deployments. This vulnerability is fixed in 2.11.6, 2.10.15, and 2.9.20.

Affected packages

Versions sourced from the GitHub Security Advisory.

PackageAffected versionsPatched versions
github.com/argoproj/argo-cdGo
>= 1.0.0, <= 1.8.7
github.com/argoproj/argo-cd/v2Go
< 2.9.202.9.20
github.com/argoproj/argo-cd/v2Go
>= 2.10.0, < 2.10.152.10.15
github.com/argoproj/argo-cd/v2Go
>= 2.11.0, < 2.11.62.11.6

Affected products

1

Patches

6
46c0c0b64dea

Merge commit from fork

https://github.com/argoproj/argo-cdpasha-codefreshJul 22, 2024via ghsa
6 files changed · +80 11
  • docs/operator-manual/argocd-cm.yaml+2 0 modified
    @@ -412,3 +412,5 @@ data:
                   cluster:
                     name: some-cluster
                     server: https://some-cluster
    +  # The maximum size of the payload that can be sent to the webhook server.
    +  webhook.maxPayloadSizeMB: 1024
    \ No newline at end of file
    
  • docs/operator-manual/webhook.md+2 0 modified
    @@ -19,6 +19,8 @@ URL configured in the Git provider should use the `/api/webhook` endpoint of you
     (e.g. `https://argocd.example.com/api/webhook`). If you wish to use a shared secret, input an
     arbitrary value in the secret. This value will be used when configuring the webhook in the next step.
     
    +To prevent DDoS attacks with unauthenticated webhook events (the `/api/webhook` endpoint currently lacks rate limiting protection), it is recommended to limit the payload size. You can achieve this by configuring the `argocd-cm` ConfigMap with the `webhook.maxPayloadSizeMB` attribute. The default value is 1GB.
    +
     ## Github
     
     ![Add Webhook](../assets/webhook-config.png "Add Webhook")
    
  • server/server.go+1 1 modified
    @@ -1034,7 +1034,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl
     
     	// Webhook handler for git events (Note: cache timeouts are hardcoded because API server does not write to cache and not really using them)
     	argoDB := db.NewDB(a.Namespace, a.settingsMgr, a.KubeClientset)
    -	acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, repocache.NewCache(a.Cache.GetCache(), 24*time.Hour, 3*time.Minute), a.Cache, argoDB)
    +	acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, repocache.NewCache(a.Cache.GetCache(), 24*time.Hour, 3*time.Minute), a.Cache, argoDB, a.settingsMgr.GetMaxWebhookPayloadSize())
     
     	mux.HandleFunc("/api/webhook", acdWebhookHandler.Handler)
     
    
  • util/settings/settings.go+30 6 modified
    @@ -429,6 +429,8 @@ const (
     	settingsWebhookAzureDevOpsUsernameKey = "webhook.azuredevops.username"
     	// settingsWebhookAzureDevOpsPasswordKey is the key for Azure DevOps webhook password
     	settingsWebhookAzureDevOpsPasswordKey = "webhook.azuredevops.password"
    +	// settingsWebhookMaxPayloadSize is the key for the maximum payload size for webhooks in MB
    +	settingsWebhookMaxPayloadSizeMB = "webhook.maxPayloadSizeMB"
     	// settingsApplicationInstanceLabelKey is the key to configure injected app instance label key
     	settingsApplicationInstanceLabelKey = "application.instanceLabelKey"
     	// settingsResourceTrackingMethodKey is the key to configure tracking method for application resources
    @@ -506,14 +508,17 @@ const (
     	RespectRBACValueNormal = "normal"
     )
     
    -var (
    -	sourceTypeToEnableGenerationKey = map[v1alpha1.ApplicationSourceType]string{
    -		v1alpha1.ApplicationSourceTypeKustomize: "kustomize.enable",
    -		v1alpha1.ApplicationSourceTypeHelm:      "helm.enable",
    -		v1alpha1.ApplicationSourceTypeDirectory: "jsonnet.enable",
    -	}
    +const (
    +	// default max webhook payload size is 1GB
    +	defaultMaxWebhookPayloadSize = int64(1) * 1024 * 1024 * 1024
     )
     
    +var sourceTypeToEnableGenerationKey = map[v1alpha1.ApplicationSourceType]string{
    +	v1alpha1.ApplicationSourceTypeKustomize: "kustomize.enable",
    +	v1alpha1.ApplicationSourceTypeHelm:      "helm.enable",
    +	v1alpha1.ApplicationSourceTypeDirectory: "jsonnet.enable",
    +}
    +
     // SettingsManager holds config info for a new manager with which to access Kubernetes ConfigMaps.
     type SettingsManager struct {
     	ctx             context.Context
    @@ -2209,3 +2214,22 @@ func (mgr *SettingsManager) GetResourceCustomLabels() ([]string, error) {
     	}
     	return []string{}, nil
     }
    +
    +func (mgr *SettingsManager) GetMaxWebhookPayloadSize() int64 {
    +	argoCDCM, err := mgr.getConfigMap()
    +	if err != nil {
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	if argoCDCM.Data[settingsWebhookMaxPayloadSizeMB] == "" {
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	maxPayloadSizeMB, err := strconv.ParseInt(argoCDCM.Data[settingsWebhookMaxPayloadSizeMB], 10, 64)
    +	if err != nil {
    +		log.Warnf("Failed to parse '%s' key: %v", settingsWebhookMaxPayloadSizeMB, err)
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	return maxPayloadSizeMB * 1024 * 1024
    +}
    
  • util/webhook/webhook.go+17 1 modified
    @@ -42,6 +42,8 @@ type settingsSource interface {
     // https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36
     const usernameRegex = `[a-zA-Z0-9_\.][a-zA-Z0-9_\.-]{0,30}[a-zA-Z0-9_\.\$-]?`
     
    +const payloadQueueSize = 50000
    +
     var (
     	_                              settingsSource = &settings.SettingsManager{}
     	errBasicAuthVerificationFailed                = errors.New("basic auth verification failed")
    @@ -62,9 +64,11 @@ type ArgoCDWebhookHandler struct {
     	azuredevopsAuthHandler func(r *http.Request) error
     	gogs                   *gogs.Webhook
     	settingsSrc            settingsSource
    +	queue                  chan interface{}
    +	maxWebhookPayloadSizeB int64
     }
     
    -func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB) *ArgoCDWebhookHandler {
    +func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB, maxWebhookPayloadSizeB int64) *ArgoCDWebhookHandler {
     	githubWebhook, err := github.New(github.Options.Secret(set.WebhookGitHubSecret))
     	if err != nil {
     		log.Warnf("Unable to init the GitHub webhook")
    @@ -114,6 +118,8 @@ func NewHandler(namespace string, applicationNamespaces []string, appClientset a
     		repoCache:              repoCache,
     		serverCache:            serverCache,
     		db:                     argoDB,
    +		queue:                  make(chan interface{}, payloadQueueSize),
    +		maxWebhookPayloadSizeB: maxWebhookPayloadSizeB,
     	}
     
     	return &acdWebhook
    @@ -458,6 +464,8 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
     	var payload interface{}
     	var err error
     
    +	r.Body = http.MaxBytesReader(w, r.Body, a.maxWebhookPayloadSizeB)
    +
     	switch {
     	case r.Header.Get("X-Vss-Activityid") != "":
     		if err = a.azuredevopsAuthHandler(r); err != nil {
    @@ -500,6 +508,14 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
     	}
     
     	if err != nil {
    +		// If the error is due to a large payload, return a more user-friendly error message
    +		if err.Error() == "error parsing payload" {
    +			msg := fmt.Sprintf("Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under %v MB) and ensure it is valid JSON", a.maxWebhookPayloadSizeB/1024/1024)
    +			log.WithField(common.SecurityField, common.SecurityHigh).Warn(msg)
    +			http.Error(w, msg, http.StatusBadRequest)
    +			return
    +		}
    +
     		log.Infof("Webhook processing failed: %s", err)
     		status := http.StatusBadRequest
     		if r.Method != http.MethodPost {
    
  • util/webhook/webhook_test.go+28 3 modified
    @@ -4,6 +4,7 @@ import (
     	"bytes"
     	"encoding/json"
     	"fmt"
    +	"github.com/stretchr/testify/require"
     	"io"
     	"net/http"
     	"net/http/httptest"
    @@ -56,6 +57,11 @@ type reactorDef struct {
     }
     
     func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects ...runtime.Object) *ArgoCDWebhookHandler {
    +	defaultMaxPayloadSize := int64(1) * 1024 * 1024 * 1024
    +	return NewMockHandlerWithPayloadLimit(reactor, applicationNamespaces, defaultMaxPayloadSize, objects...)
    +}
    +
    +func NewMockHandlerWithPayloadLimit(reactor *reactorDef, applicationNamespaces []string, maxPayloadSize int64, objects ...runtime.Object) *ArgoCDWebhookHandler {
     	appClientset := appclientset.NewSimpleClientset(objects...)
     	if reactor != nil {
     		defaultReactor := appClientset.ReactionChain[0]
    @@ -71,7 +77,7 @@ func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects
     		cacheClient,
     		1*time.Minute,
     		1*time.Minute,
    -	), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{})
    +	), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{}, maxPayloadSize)
     }
     
     func TestGitHubCommitEvent(t *testing.T) {
    @@ -391,8 +397,9 @@ func TestInvalidEvent(t *testing.T) {
     	req.Header.Set("X-GitHub-Event", "push")
     	w := httptest.NewRecorder()
     	h.Handler(w, req)
    -	assert.Equal(t, w.Code, http.StatusBadRequest)
    -	expectedLogResult := "Webhook processing failed: error parsing payload"
    +	close(h.queue)
    +	assert.Equal(t, http.StatusBadRequest, w.Code)
    +	expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 1024 MB) and ensure it is valid JSON"
     	assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
     	assert.Equal(t, expectedLogResult+"\n", w.Body.String())
     	hook.Reset()
    @@ -683,3 +690,21 @@ func Test_getWebUrlRegex(t *testing.T) {
     		})
     	}
     }
    +
    +func TestGitHubCommitEventMaxPayloadSize(t *testing.T) {
    +	hook := test.NewGlobal()
    +	maxPayloadSize := int64(100)
    +	h := NewMockHandlerWithPayloadLimit(nil, []string{}, maxPayloadSize)
    +	req := httptest.NewRequest(http.MethodPost, "/api/webhook", nil)
    +	req.Header.Set("X-GitHub-Event", "push")
    +	eventJSON, err := os.ReadFile("testdata/github-commit-event.json")
    +	require.NoError(t, err)
    +	req.Body = io.NopCloser(bytes.NewReader(eventJSON))
    +	w := httptest.NewRecorder()
    +	h.Handler(w, req)
    +	close(h.queue)
    +	assert.Equal(t, http.StatusBadRequest, w.Code)
    +	expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 0 MB) and ensure it is valid JSON"
    +	assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
    +	hook.Reset()
    +}
    
d881ee78949e

Merge commit from fork

https://github.com/argoproj/argo-cdpasha-codefreshJul 22, 2024via ghsa
6 files changed · +80 11
  • docs/operator-manual/argocd-cm.yaml+2 0 modified
    @@ -406,3 +406,5 @@ data:
                   cluster:
                     name: some-cluster
                     server: https://some-cluster
    +  # The maximum size of the payload that can be sent to the webhook server.
    +  webhook.maxPayloadSizeMB: 1024
    \ No newline at end of file
    
  • docs/operator-manual/webhook.md+2 0 modified
    @@ -19,6 +19,8 @@ URL configured in the Git provider should use the `/api/webhook` endpoint of you
     (e.g. `https://argocd.example.com/api/webhook`). If you wish to use a shared secret, input an
     arbitrary value in the secret. This value will be used when configuring the webhook in the next step.
     
    +To prevent DDoS attacks with unauthenticated webhook events (the `/api/webhook` endpoint currently lacks rate limiting protection), it is recommended to limit the payload size. You can achieve this by configuring the `argocd-cm` ConfigMap with the `webhook.maxPayloadSizeMB` attribute. The default value is 1GB.
    +
     ## Github
     
     ![Add Webhook](../assets/webhook-config.png "Add Webhook")
    
  • server/server.go+1 1 modified
    @@ -1034,7 +1034,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl
     
     	// Webhook handler for git events (Note: cache timeouts are hardcoded because API server does not write to cache and not really using them)
     	argoDB := db.NewDB(a.Namespace, a.settingsMgr, a.KubeClientset)
    -	acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, repocache.NewCache(a.Cache.GetCache(), 24*time.Hour, 3*time.Minute), a.Cache, argoDB)
    +	acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, repocache.NewCache(a.Cache.GetCache(), 24*time.Hour, 3*time.Minute), a.Cache, argoDB, a.settingsMgr.GetMaxWebhookPayloadSize())
     
     	mux.HandleFunc("/api/webhook", acdWebhookHandler.Handler)
     
    
  • util/settings/settings.go+30 6 modified
    @@ -420,6 +420,8 @@ const (
     	settingsWebhookAzureDevOpsUsernameKey = "webhook.azuredevops.username"
     	// settingsWebhookAzureDevOpsPasswordKey is the key for Azure DevOps webhook password
     	settingsWebhookAzureDevOpsPasswordKey = "webhook.azuredevops.password"
    +	// settingsWebhookMaxPayloadSize is the key for the maximum payload size for webhooks in MB
    +	settingsWebhookMaxPayloadSizeMB = "webhook.maxPayloadSizeMB"
     	// settingsApplicationInstanceLabelKey is the key to configure injected app instance label key
     	settingsApplicationInstanceLabelKey = "application.instanceLabelKey"
     	// settingsResourceTrackingMethodKey is the key to configure tracking method for application resources
    @@ -497,14 +499,17 @@ const (
     	RespectRBACValueNormal = "normal"
     )
     
    -var (
    -	sourceTypeToEnableGenerationKey = map[v1alpha1.ApplicationSourceType]string{
    -		v1alpha1.ApplicationSourceTypeKustomize: "kustomize.enable",
    -		v1alpha1.ApplicationSourceTypeHelm:      "helm.enable",
    -		v1alpha1.ApplicationSourceTypeDirectory: "jsonnet.enable",
    -	}
    +const (
    +	// default max webhook payload size is 1GB
    +	defaultMaxWebhookPayloadSize = int64(1) * 1024 * 1024 * 1024
     )
     
    +var sourceTypeToEnableGenerationKey = map[v1alpha1.ApplicationSourceType]string{
    +	v1alpha1.ApplicationSourceTypeKustomize: "kustomize.enable",
    +	v1alpha1.ApplicationSourceTypeHelm:      "helm.enable",
    +	v1alpha1.ApplicationSourceTypeDirectory: "jsonnet.enable",
    +}
    +
     // SettingsManager holds config info for a new manager with which to access Kubernetes ConfigMaps.
     type SettingsManager struct {
     	ctx             context.Context
    @@ -2159,3 +2164,22 @@ func (mgr *SettingsManager) GetResourceCustomLabels() ([]string, error) {
     	}
     	return []string{}, nil
     }
    +
    +func (mgr *SettingsManager) GetMaxWebhookPayloadSize() int64 {
    +	argoCDCM, err := mgr.getConfigMap()
    +	if err != nil {
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	if argoCDCM.Data[settingsWebhookMaxPayloadSizeMB] == "" {
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	maxPayloadSizeMB, err := strconv.ParseInt(argoCDCM.Data[settingsWebhookMaxPayloadSizeMB], 10, 64)
    +	if err != nil {
    +		log.Warnf("Failed to parse '%s' key: %v", settingsWebhookMaxPayloadSizeMB, err)
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	return maxPayloadSizeMB * 1024 * 1024
    +}
    
  • util/webhook/webhook.go+17 1 modified
    @@ -42,6 +42,8 @@ type settingsSource interface {
     // https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36
     const usernameRegex = `[a-zA-Z0-9_\.][a-zA-Z0-9_\.-]{0,30}[a-zA-Z0-9_\.\$-]?`
     
    +const payloadQueueSize = 50000
    +
     var (
     	_                              settingsSource = &settings.SettingsManager{}
     	errBasicAuthVerificationFailed                = errors.New("basic auth verification failed")
    @@ -62,9 +64,11 @@ type ArgoCDWebhookHandler struct {
     	azuredevopsAuthHandler func(r *http.Request) error
     	gogs                   *gogs.Webhook
     	settingsSrc            settingsSource
    +	queue                  chan interface{}
    +	maxWebhookPayloadSizeB int64
     }
     
    -func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB) *ArgoCDWebhookHandler {
    +func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB, maxWebhookPayloadSizeB int64) *ArgoCDWebhookHandler {
     	githubWebhook, err := github.New(github.Options.Secret(set.WebhookGitHubSecret))
     	if err != nil {
     		log.Warnf("Unable to init the GitHub webhook")
    @@ -114,6 +118,8 @@ func NewHandler(namespace string, applicationNamespaces []string, appClientset a
     		repoCache:              repoCache,
     		serverCache:            serverCache,
     		db:                     argoDB,
    +		queue:                  make(chan interface{}, payloadQueueSize),
    +		maxWebhookPayloadSizeB: maxWebhookPayloadSizeB,
     	}
     
     	return &acdWebhook
    @@ -458,6 +464,8 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
     	var payload interface{}
     	var err error
     
    +	r.Body = http.MaxBytesReader(w, r.Body, a.maxWebhookPayloadSizeB)
    +
     	switch {
     	case r.Header.Get("X-Vss-Activityid") != "":
     		if err = a.azuredevopsAuthHandler(r); err != nil {
    @@ -500,6 +508,14 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
     	}
     
     	if err != nil {
    +		// If the error is due to a large payload, return a more user-friendly error message
    +		if err.Error() == "error parsing payload" {
    +			msg := fmt.Sprintf("Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under %v MB) and ensure it is valid JSON", a.maxWebhookPayloadSizeB/1024/1024)
    +			log.WithField(common.SecurityField, common.SecurityHigh).Warn(msg)
    +			http.Error(w, msg, http.StatusBadRequest)
    +			return
    +		}
    +
     		log.Infof("Webhook processing failed: %s", err)
     		status := http.StatusBadRequest
     		if r.Method != http.MethodPost {
    
  • util/webhook/webhook_test.go+28 3 modified
    @@ -4,6 +4,7 @@ import (
     	"bytes"
     	"encoding/json"
     	"fmt"
    +	"github.com/stretchr/testify/require"
     	"io"
     	"net/http"
     	"net/http/httptest"
    @@ -56,6 +57,11 @@ type reactorDef struct {
     }
     
     func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects ...runtime.Object) *ArgoCDWebhookHandler {
    +	defaultMaxPayloadSize := int64(1) * 1024 * 1024 * 1024
    +	return NewMockHandlerWithPayloadLimit(reactor, applicationNamespaces, defaultMaxPayloadSize, objects...)
    +}
    +
    +func NewMockHandlerWithPayloadLimit(reactor *reactorDef, applicationNamespaces []string, maxPayloadSize int64, objects ...runtime.Object) *ArgoCDWebhookHandler {
     	appClientset := appclientset.NewSimpleClientset(objects...)
     	if reactor != nil {
     		defaultReactor := appClientset.ReactionChain[0]
    @@ -71,7 +77,7 @@ func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects
     		cacheClient,
     		1*time.Minute,
     		1*time.Minute,
    -	), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{})
    +	), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{}, maxPayloadSize)
     }
     
     func TestGitHubCommitEvent(t *testing.T) {
    @@ -391,8 +397,9 @@ func TestInvalidEvent(t *testing.T) {
     	req.Header.Set("X-GitHub-Event", "push")
     	w := httptest.NewRecorder()
     	h.Handler(w, req)
    -	assert.Equal(t, w.Code, http.StatusBadRequest)
    -	expectedLogResult := "Webhook processing failed: error parsing payload"
    +	close(h.queue)
    +	assert.Equal(t, http.StatusBadRequest, w.Code)
    +	expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 1024 MB) and ensure it is valid JSON"
     	assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
     	assert.Equal(t, expectedLogResult+"\n", w.Body.String())
     	hook.Reset()
    @@ -683,3 +690,21 @@ func Test_getWebUrlRegex(t *testing.T) {
     		})
     	}
     }
    +
    +func TestGitHubCommitEventMaxPayloadSize(t *testing.T) {
    +	hook := test.NewGlobal()
    +	maxPayloadSize := int64(100)
    +	h := NewMockHandlerWithPayloadLimit(nil, []string{}, maxPayloadSize)
    +	req := httptest.NewRequest(http.MethodPost, "/api/webhook", nil)
    +	req.Header.Set("X-GitHub-Event", "push")
    +	eventJSON, err := os.ReadFile("testdata/github-commit-event.json")
    +	require.NoError(t, err)
    +	req.Body = io.NopCloser(bytes.NewReader(eventJSON))
    +	w := httptest.NewRecorder()
    +	h.Handler(w, req)
    +	close(h.queue)
    +	assert.Equal(t, http.StatusBadRequest, w.Code)
    +	expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 0 MB) and ensure it is valid JSON"
    +	assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
    +	hook.Reset()
    +}
    
540e3a57b90e

Merge commit from fork

https://github.com/argoproj/argo-cdpasha-codefreshJul 22, 2024via ghsa
6 files changed · +76 5
  • docs/operator-manual/argocd-cm.yaml+3 0 modified
    @@ -410,3 +410,6 @@ data:
                   cluster:
                     name: some-cluster
                     server: https://some-cluster
    +          
    +          # The maximum size of the payload that can be sent to the webhook server.
    +          webhook.maxPayloadSizeMB: 1024
    \ No newline at end of file
    
  • docs/operator-manual/webhook.md+1 0 modified
    @@ -19,6 +19,7 @@ URL configured in the Git provider should use the `/api/webhook` endpoint of you
     (e.g. `https://argocd.example.com/api/webhook`). If you wish to use a shared secret, input an
     arbitrary value in the secret. This value will be used when configuring the webhook in the next step.
     
    +To prevent DDoS attacks with unauthenticated webhook events (the `/api/webhook` endpoint currently lacks rate limiting protection), it is recommended to limit the payload size. You can achieve this by configuring the `argocd-cm` ConfigMap with the `webhook.maxPayloadSizeMB` attribute. The default value is 1GB.
     ## Github
     
     ![Add Webhook](../assets/webhook-config.png "Add Webhook")
    
  • server/server.go+1 1 modified
    @@ -1048,7 +1048,7 @@ func (a *ArgoCDServer) newHTTPServer(ctx context.Context, port int, grpcWebHandl
     
     	// Webhook handler for git events (Note: cache timeouts are hardcoded because API server does not write to cache and not really using them)
     	argoDB := db.NewDB(a.Namespace, a.settingsMgr, a.KubeClientset)
    -	acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, a.RepoServerCache, a.Cache, argoDB)
    +	acdWebhookHandler := webhook.NewHandler(a.Namespace, a.ArgoCDServerOpts.ApplicationNamespaces, a.AppClientset, a.settings, a.settingsMgr, a.RepoServerCache, a.Cache, argoDB, a.settingsMgr.GetMaxWebhookPayloadSize())
     
     	mux.HandleFunc("/api/webhook", acdWebhookHandler.Handler)
     
    
  • util/settings/settings.go+26 0 modified
    @@ -431,6 +431,8 @@ const (
     	settingsWebhookAzureDevOpsUsernameKey = "webhook.azuredevops.username"
     	// settingsWebhookAzureDevOpsPasswordKey is the key for Azure DevOps webhook password
     	settingsWebhookAzureDevOpsPasswordKey = "webhook.azuredevops.password"
    +	// settingsWebhookMaxPayloadSize is the key for the maximum payload size for webhooks in MB
    +	settingsWebhookMaxPayloadSizeMB = "webhook.maxPayloadSizeMB"
     	// settingsApplicationInstanceLabelKey is the key to configure injected app instance label key
     	settingsApplicationInstanceLabelKey = "application.instanceLabelKey"
     	// settingsResourceTrackingMethodKey is the key to configure tracking method for application resources
    @@ -518,6 +520,11 @@ var (
     	}
     )
     
    +const (
    +	// default max webhook payload size is 1GB
    +	defaultMaxWebhookPayloadSize = int64(1) * 1024 * 1024 * 1024
    +)
    +
     // SettingsManager holds config info for a new manager with which to access Kubernetes ConfigMaps.
     type SettingsManager struct {
     	ctx             context.Context
    @@ -2221,3 +2228,22 @@ func (mgr *SettingsManager) GetResourceCustomLabels() ([]string, error) {
     	}
     	return []string{}, nil
     }
    +
    +func (mgr *SettingsManager) GetMaxWebhookPayloadSize() int64 {
    +	argoCDCM, err := mgr.getConfigMap()
    +	if err != nil {
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	if argoCDCM.Data[settingsWebhookMaxPayloadSizeMB] == "" {
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	maxPayloadSizeMB, err := strconv.ParseInt(argoCDCM.Data[settingsWebhookMaxPayloadSizeMB], 10, 64)
    +	if err != nil {
    +		log.Warnf("Failed to parse '%s' key: %v", settingsWebhookMaxPayloadSizeMB, err)
    +		return defaultMaxWebhookPayloadSize
    +	}
    +
    +	return maxPayloadSizeMB * 1024 * 1024
    +}
    
  • util/webhook/webhook.go+17 1 modified
    @@ -41,6 +41,8 @@ type settingsSource interface {
     // https://github.com/shadow-maint/shadow/blob/master/libmisc/chkname.c#L36
     const usernameRegex = `[a-zA-Z0-9_\.][a-zA-Z0-9_\.-]{0,30}[a-zA-Z0-9_\.\$-]?`
     
    +const payloadQueueSize = 50000
    +
     var (
     	_                              settingsSource = &settings.SettingsManager{}
     	errBasicAuthVerificationFailed                = errors.New("basic auth verification failed")
    @@ -61,9 +63,11 @@ type ArgoCDWebhookHandler struct {
     	azuredevopsAuthHandler func(r *http.Request) error
     	gogs                   *gogs.Webhook
     	settingsSrc            settingsSource
    +	queue                  chan interface{}
    +	maxWebhookPayloadSizeB int64
     }
     
    -func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB) *ArgoCDWebhookHandler {
    +func NewHandler(namespace string, applicationNamespaces []string, appClientset appclientset.Interface, set *settings.ArgoCDSettings, settingsSrc settingsSource, repoCache *cache.Cache, serverCache *servercache.Cache, argoDB db.ArgoDB, maxWebhookPayloadSizeB int64) *ArgoCDWebhookHandler {
     	githubWebhook, err := github.New(github.Options.Secret(set.WebhookGitHubSecret))
     	if err != nil {
     		log.Warnf("Unable to init the GitHub webhook")
    @@ -113,6 +117,8 @@ func NewHandler(namespace string, applicationNamespaces []string, appClientset a
     		repoCache:              repoCache,
     		serverCache:            serverCache,
     		db:                     argoDB,
    +		queue:                  make(chan interface{}, payloadQueueSize),
    +		maxWebhookPayloadSizeB: maxWebhookPayloadSizeB,
     	}
     
     	return &acdWebhook
    @@ -388,6 +394,8 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
     	var payload interface{}
     	var err error
     
    +	r.Body = http.MaxBytesReader(w, r.Body, a.maxWebhookPayloadSizeB)
    +
     	switch {
     	case r.Header.Get("X-Vss-Activityid") != "":
     		if err = a.azuredevopsAuthHandler(r); err != nil {
    @@ -430,6 +438,14 @@ func (a *ArgoCDWebhookHandler) Handler(w http.ResponseWriter, r *http.Request) {
     	}
     
     	if err != nil {
    +		// If the error is due to a large payload, return a more user-friendly error message
    +		if err.Error() == "error parsing payload" {
    +			msg := fmt.Sprintf("Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under %v MB) and ensure it is valid JSON", a.maxWebhookPayloadSizeB/1024/1024)
    +			log.WithField(common.SecurityField, common.SecurityHigh).Warn(msg)
    +			http.Error(w, msg, http.StatusBadRequest)
    +			return
    +		}
    +
     		log.Infof("Webhook processing failed: %s", err)
     		status := http.StatusBadRequest
     		if r.Method != http.MethodPost {
    
  • util/webhook/webhook_test.go+28 3 modified
    @@ -4,6 +4,7 @@ import (
     	"bytes"
     	"encoding/json"
     	"fmt"
    +	"github.com/stretchr/testify/require"
     	"io"
     	"net/http"
     	"net/http/httptest"
    @@ -56,6 +57,11 @@ type reactorDef struct {
     }
     
     func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects ...runtime.Object) *ArgoCDWebhookHandler {
    +	defaultMaxPayloadSize := int64(1) * 1024 * 1024 * 1024
    +	return NewMockHandlerWithPayloadLimit(reactor, applicationNamespaces, defaultMaxPayloadSize, objects...)
    +}
    +
    +func NewMockHandlerWithPayloadLimit(reactor *reactorDef, applicationNamespaces []string, maxPayloadSize int64, objects ...runtime.Object) *ArgoCDWebhookHandler {
     	appClientset := appclientset.NewSimpleClientset(objects...)
     	if reactor != nil {
     		defaultReactor := appClientset.ReactionChain[0]
    @@ -72,7 +78,7 @@ func NewMockHandler(reactor *reactorDef, applicationNamespaces []string, objects
     		1*time.Minute,
     		1*time.Minute,
     		10*time.Second,
    -	), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{})
    +	), servercache.NewCache(appstate.NewCache(cacheClient, time.Minute), time.Minute, time.Minute, time.Minute), &mocks.ArgoDB{}, maxPayloadSize)
     }
     
     func TestGitHubCommitEvent(t *testing.T) {
    @@ -392,8 +398,9 @@ func TestInvalidEvent(t *testing.T) {
     	req.Header.Set("X-GitHub-Event", "push")
     	w := httptest.NewRecorder()
     	h.Handler(w, req)
    -	assert.Equal(t, w.Code, http.StatusBadRequest)
    -	expectedLogResult := "Webhook processing failed: error parsing payload"
    +	close(h.queue)
    +	assert.Equal(t, http.StatusBadRequest, w.Code)
    +	expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 1024 MB) and ensure it is valid JSON"
     	assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
     	assert.Equal(t, expectedLogResult+"\n", w.Body.String())
     	hook.Reset()
    @@ -604,3 +611,21 @@ func Test_getWebUrlRegex(t *testing.T) {
     		})
     	}
     }
    +
    +func TestGitHubCommitEventMaxPayloadSize(t *testing.T) {
    +	hook := test.NewGlobal()
    +	maxPayloadSize := int64(100)
    +	h := NewMockHandlerWithPayloadLimit(nil, []string{}, maxPayloadSize)
    +	req := httptest.NewRequest(http.MethodPost, "/api/webhook", nil)
    +	req.Header.Set("X-GitHub-Event", "push")
    +	eventJSON, err := os.ReadFile("testdata/github-commit-event.json")
    +	require.NoError(t, err)
    +	req.Body = io.NopCloser(bytes.NewReader(eventJSON))
    +	w := httptest.NewRecorder()
    +	h.Handler(w, req)
    +	close(h.queue)
    +	assert.Equal(t, http.StatusBadRequest, w.Code)
    +	expectedLogResult := "Webhook processing failed: The payload is either too large or corrupted. Please check the payload size (must be under 0 MB) and ensure it is valid JSON"
    +	assert.Equal(t, expectedLogResult, hook.LastEntry().Message)
    +	hook.Reset()
    +}
    

Vulnerability mechanics

Generated by null/stub on May 9, 2026. Inputs: CWE entries + fix-commit diffs from this CVE's patches. Citations validated against bundle.

References

7

News mentions

0

No linked articles in our index yet.