CVE-2023-23947: Incorrect Authorization (CWE-863)
value: Argo CD is a declarative, GitOps continuous delivery tool for Kubernetes. All Argo CD versions starting with 2.3.0-rc1 and prior to 2.3.17, 2.4.23  2.5.11, and 2.6.2 are vulnerable to an improper authorization bug which allows users who have the ability to update at least one cluster secret to update any cluster secret. The attacker could use this access to escalate privileges (potentially controlling Kubernetes resources) or to break Argo CD functionality (by preventing connections to external clusters). A patch for this vulnerability has been released in Argo CD versions 2.6.2, 2.5.11, 2.4.23, and 2.3.17. Two workarounds are available. Either modify the RBAC configuration to completely revoke all `clusters, update` access, or use the `destinations` and `clusterResourceWhitelist` fields to apply similar restrictions as the `namespaces` and `clusterResources` fields.
CWE description: The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
CVSS v3 base score: 8.5
Published: 2023-02-16T18:15Z
Repository: https://github.com/argoproj/argo-cd
Language: Go
Files changed: common/common.go, server/cluster/cluster.go, server/cluster/cluster_test.go
Commit date: 2023-02-16 09:07:57 -0500
Commit message: Merge pull request from GHSA-3jfq-742w-xg8j

fix test name

Signed-off-by: Michael Crenshaw <350466+crenshaw-dev@users.noreply.github.com>
Diff: diff --git a/common/common.go b/common/common.go
index c026ab3c2..c22064e94 100644
--- a/common/common.go
+++ b/common/common.go
@@ -6,10 +6,12 @@ import (
 	"path/filepath"
 	"strconv"
 	"time"
 
 	"github.com/sirupsen/logrus"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // Default service addresses and URLS of Argo CD internal services
 const (
 	// DefaultRepoServerAddr is the gRPC address of the Argo CD repo server
@@ -314,5 +316,7 @@ const (
 
 // Common error messages
 const TokenVerificationError = "failed to verify the token"
 
 var TokenVerificationErr = errors.New(TokenVerificationError)
+
+var PermissionDeniedAPIError = status.Error(codes.PermissionDenied, "permission denied")
diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go
index 2225a5d6f..f39396f0c 100644
--- a/server/cluster/cluster.go
+++ b/server/cluster/cluster.go
@@ -1,21 +1,21 @@
 package cluster
 
 import (
+	"context"
 	"net/url"
 	"time"
 
-	"context"
-
 	"github.com/argoproj/gitops-engine/pkg/utils/kube"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes"
 
+	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
 	appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	servercache "github.com/argoproj/argo-cd/v2/server/cache"
 	"github.com/argoproj/argo-cd/v2/server/rbacpolicy"
 	"github.com/argoproj/argo-cd/v2/util/argo"
@@ -133,11 +133,11 @@ func (s *Server) Get(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Clust
 }
 
 func (s *Server) getClusterWith403IfNotExist(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Cluster, error) {
 	repo, err := s.getCluster(ctx, q)
 	if err != nil || repo == nil {
-		return nil, status.Error(codes.PermissionDenied, "permission denied")
+		return nil, common.PermissionDeniedAPIError
 	}
 	return repo, nil
 }
 
 func (s *Server) getCluster(ctx context.Context, q *cluster.ClusterQuery) (*appv1.Cluster, error) {
@@ -219,18 +219,18 @@ func (s *Server) Update(ctx context.Context, q *cluster.ClusterUpdateRequest) (*
 	if err != nil {
 		return nil, err
 	}
 
 	// verify that user can do update inside project where cluster is located
-	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(c.Project, q.Cluster.Server)); err != nil {
-		return nil, err
+	if !s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(c.Project, c.Server)) {
+		return nil, common.PermissionDeniedAPIError
 	}
 
 	if len(q.UpdatedFields) == 0 || sets.NewString(q.UpdatedFields...).Has("project") {
 		// verify that user can do update inside project where cluster will be located
-		if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(q.Cluster.Project, q.Cluster.Server)); err != nil {
-			return nil, err
+		if !s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(q.Cluster.Project, c.Server)) {
+			return nil, common.PermissionDeniedAPIError
 		}
 	}
 
 	if len(q.UpdatedFields) != 0 {
 		for _, path := range q.UpdatedFields {
diff --git a/server/cluster/cluster_test.go b/server/cluster/cluster_test.go
index dec7d97b8..c5b07a4ee 100644
--- a/server/cluster/cluster_test.go
+++ b/server/cluster/cluster_test.go
@@ -1,10 +1,11 @@
 package cluster
 
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"testing"
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -47,10 +48,121 @@ func newNoopEnforcer() *rbac.Enforcer {
 	enf := rbac.NewEnforcer(fake.NewSimpleClientset(test.NewFakeConfigMap()), test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil)
 	enf.EnableEnforce(false)
 	return enf
 }
 
+func TestUpdateCluster_RejectInvalidParams(t *testing.T) {
+	testCases := []struct {
+		name    string
+		request clusterapi.ClusterUpdateRequest
+	}{
+		{
+			name:    "allowed cluster URL in body, disallowed cluster URL in query",
+			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "", Value: "https://127.0.0.2"}, UpdatedFields: []string{"clusterResources", "project"}},
+		},
+		{
+			name:    "allowed cluster URL in body, disallowed cluster name in query",
+			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "name", Value: "disallowed-unscoped"}, UpdatedFields: []string{"clusterResources", "project"}},
+		},
+		{
+			name:    "allowed cluster URL in body, disallowed cluster name in query, changing unscoped to scoped",
+			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "allowed-project", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "", Value: "https://127.0.0.2"}, UpdatedFields: []string{"clusterResources", "project"}},
+		},
+		{
+			name:    "allowed cluster URL in body, disallowed cluster URL in query, changing unscoped to scoped",
+			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "allowed-project", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "name", Value: "disallowed-unscoped"}, UpdatedFields: []string{"clusterResources", "project"}},
+		},
+	}
+
+	db := &dbmocks.ArgoDB{}
+
+	clusters := []v1alpha1.Cluster{
+		{
+			Name:   "allowed-unscoped",
+			Server: "https://127.0.0.1",
+		},
+		{
+			Name:   "disallowed-unscoped",
+			Server: "https://127.0.0.2",
+		},
+		{
+			Name:    "allowed-scoped",
+			Server:  "https://127.0.0.3",
+			Project: "allowed-project",
+		},
+		{
+			Name:    "disallowed-scoped",
+			Server:  "https://127.0.0.4",
+			Project: "disallowed-project",
+		},
+	}
+
+	db.On("ListClusters", mock.Anything).Return(
+		func(ctx context.Context) *v1alpha1.ClusterList {
+			return &v1alpha1.ClusterList{
+				ListMeta: v1.ListMeta{},
+				Items:    clusters,
+			}
+		},
+		func(ctx context.Context) error {
+			return nil
+		},
+	)
+	db.On("UpdateCluster", mock.Anything, mock.Anything).Return(
+		func(ctx context.Context, c *v1alpha1.Cluster) *v1alpha1.Cluster {
+			for _, cluster := range clusters {
+				if c.Server == cluster.Server {
+					return c
+				}
+			}
+			return nil
+		},
+		func(ctx context.Context, c *v1alpha1.Cluster) error {
+			for _, cluster := range clusters {
+				if c.Server == cluster.Server {
+					return nil
+				}
+			}
+			return fmt.Errorf("cluster '%s' not found", c.Server)
+		},
+	)
+	db.On("GetCluster", mock.Anything, mock.Anything).Return(
+		func(ctx context.Context, server string) *v1alpha1.Cluster {
+			for _, cluster := range clusters {
+				if server == cluster.Server {
+					return &cluster
+				}
+			}
+			return nil
+		},
+		func(ctx context.Context, server string) error {
+			for _, cluster := range clusters {
+				if server == cluster.Server {
+					return nil
+				}
+			}
+			return fmt.Errorf("cluster '%s' not found", server)
+		},
+	)
+
+	enf := rbac.NewEnforcer(fake.NewSimpleClientset(test.NewFakeConfigMap()), test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil)
+	_ = enf.SetBuiltinPolicy(`p, role:test, clusters, *, https://127.0.0.1, allow
+p, role:test, clusters, *, allowed-project/*, allow`)
+	enf.SetDefaultRole("role:test")
+	server := NewServer(db, enf, newServerInMemoryCache(), &kubetest.MockKubectlCmd{})
+
+	for _, c := range testCases {
+		cc := c
+		t.Run(cc.name, func(t *testing.T) {
+			t.Parallel()
+			out, err := server.Update(context.Background(), &cc.request)
+			require.Nil(t, out)
+			assert.ErrorIs(t, err, common.PermissionDeniedAPIError)
+		})
+	}
+}
+
 func TestGetCluster_UrlEncodedName(t *testing.T) {
 	db := &dbmocks.ArgoDB{}
 
 	mockCluster := v1alpha1.Cluster{
 		Name:       "test/ing",

Vulnerable code: 	"context"

		return nil, status.Error(codes.PermissionDenied, "permission denied")
	if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(c.Project, q.Cluster.Server)); err != nil {
		return nil, err
		if err := s.enf.EnforceErr(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(q.Cluster.Project, q.Cluster.Server)); err != nil {
			return nil, err
Fixed code: 	"google.golang.org/grpc/codes"
	"google.golang.org/grpc/status"

var PermissionDeniedAPIError = status.Error(codes.PermissionDenied, "permission denied")
	"context"
	"github.com/argoproj/argo-cd/v2/common"
		return nil, common.PermissionDeniedAPIError
	if !s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(c.Project, c.Server)) {
		return nil, common.PermissionDeniedAPIError
		if !s.enf.Enforce(ctx.Value("claims"), rbacpolicy.ResourceClusters, rbacpolicy.ActionUpdate, createRBACObject(q.Cluster.Project, c.Server)) {
			return nil, common.PermissionDeniedAPIError
	"fmt"
func TestUpdateCluster_RejectInvalidParams(t *testing.T) {
	testCases := []struct {
		name    string
		request clusterapi.ClusterUpdateRequest
	}{
		{
			name:    "allowed cluster URL in body, disallowed cluster URL in query",
			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "", Value: "https://127.0.0.2"}, UpdatedFields: []string{"clusterResources", "project"}},
		},
		{
			name:    "allowed cluster URL in body, disallowed cluster name in query",
			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "name", Value: "disallowed-unscoped"}, UpdatedFields: []string{"clusterResources", "project"}},
		},
		{
			name:    "allowed cluster URL in body, disallowed cluster name in query, changing unscoped to scoped",
			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "allowed-project", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "", Value: "https://127.0.0.2"}, UpdatedFields: []string{"clusterResources", "project"}},
		},
		{
			name:    "allowed cluster URL in body, disallowed cluster URL in query, changing unscoped to scoped",
			request: clusterapi.ClusterUpdateRequest{Cluster: &v1alpha1.Cluster{Name: "", Server: "https://127.0.0.1", Project: "allowed-project", ClusterResources: true}, Id: &clusterapi.ClusterID{Type: "name", Value: "disallowed-unscoped"}, UpdatedFields: []string{"clusterResources", "project"}},
		},
	}

	db := &dbmocks.ArgoDB{}

	clusters := []v1alpha1.Cluster{
		{
			Name:   "allowed-unscoped",
			Server: "https://127.0.0.1",
		},
		{
			Name:   "disallowed-unscoped",
			Server: "https://127.0.0.2",
		},
		{
			Name:    "allowed-scoped",
			Server:  "https://127.0.0.3",
			Project: "allowed-project",
		},
		{
			Name:    "disallowed-scoped",
			Server:  "https://127.0.0.4",
			Project: "disallowed-project",
		},
	}

	db.On("ListClusters", mock.Anything).Return(
		func(ctx context.Context) *v1alpha1.ClusterList {
			return &v1alpha1.ClusterList{
				ListMeta: v1.ListMeta{},
				Items:    clusters,
			}
		},
		func(ctx context.Context) error {
			return nil
		},
	)
	db.On("UpdateCluster", mock.Anything, mock.Anything).Return(
		func(ctx context.Context, c *v1alpha1.Cluster) *v1alpha1.Cluster {
			for _, cluster := range clusters {
				if c.Server == cluster.Server {
					return c
				}
			}
			return nil
		},
		func(ctx context.Context, c *v1alpha1.Cluster) error {
			for _, cluster := range clusters {
				if c.Server == cluster.Server {
					return nil
				}
			}
			return fmt.Errorf("cluster '%s' not found", c.Server)
		},
	)
	db.On("GetCluster", mock.Anything, mock.Anything).Return(
		func(ctx context.Context, server string) *v1alpha1.Cluster {
			for _, cluster := range clusters {
				if server == cluster.Server {
					return &cluster
				}
			}
			return nil
		},
		func(ctx context.Context, server string) error {
			for _, cluster := range clusters {
				if server == cluster.Server {
					return nil
				}
			}
			return fmt.Errorf("cluster '%s' not found", server)
		},
	)

	enf := rbac.NewEnforcer(fake.NewSimpleClientset(test.NewFakeConfigMap()), test.FakeArgoCDNamespace, common.ArgoCDConfigMapName, nil)
	_ = enf.SetBuiltinPolicy(`p, role:test, clusters, *, https://127.0.0.1, allow
p, role:test, clusters, *, allowed-project/*, allow`)
	enf.SetDefaultRole("role:test")
	server := NewServer(db, enf, newServerInMemoryCache(), &kubetest.MockKubectlCmd{})

	for _, c := range testCases {
		cc := c
		t.Run(cc.name, func(t *testing.T) {
			t.Parallel()
			out, err := server.Update(context.Background(), &cc.request)
			require.Nil(t, out)
			assert.ErrorIs(t, err, common.PermissionDeniedAPIError)
		})
	}
}