CVE-2024-27916: Insufficient Information (NVD-CWE-noinfo)
value: Minder is a software supply chain security platform. Prior to version 0.0.33, a Minder user can use the endpoints `GetRepositoryByName`, `DeleteRepositoryByName`, and `GetArtifactByName` to access any repository in the database, irrespective of who owns the repo and any permissions present. The database query checks by repo owner, repo name and provider name (which is always `github`). These query values are not distinct for the particular user - as long as the user has valid credentials and a provider, they can set the repo owner/name to any value they want and the server will return information on this repo. Version 0.0.33 contains a patch for this issue.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-21T02:52Z
Repository: https://github.com/stacklok/minder
Language: Go
Files changed: database/query/repositories.sql, internal/controlplane/handlers_artifacts.go, internal/controlplane/handlers_repositories.go, internal/db/repositories.sql.go, internal/db/repositories_test.go
Commit date: 2024-03-04 18:39:51 +0200
Commit message: Merge pull request from GHSA-v627-69v2-xx37

Fixes #GHSA-v627-69v2-xx37

Change GetRepositoryByRepoName query to include project ID. This enforces that
the user can only access the repos in the project they authorized with.
This fixes a security issue where several API endpoints allowed any
authenticated user to manipulate any repo in the DB, irrespective of who
owned it.
Diff: diff --git a/database/query/repositories.sql b/database/query/repositories.sql
index b7c7788d..904d9a8e 100644
--- a/database/query/repositories.sql
+++ b/database/query/repositories.sql
@@ -18,11 +18,11 @@ SELECT * FROM repositories WHERE id = $1;
 
 -- name: GetRepositoryByRepoID :one
 SELECT * FROM repositories WHERE repo_id = $1;
 
 -- name: GetRepositoryByRepoName :one
-SELECT * FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3;
+SELECT * FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3 AND project_id = $4;
 
 -- name: GetRepositoryByIDAndProject :one
 SELECT * FROM repositories WHERE provider = $1 AND repo_id = $2 AND project_id = $3;
 
 -- name: ListRepositoriesByProjectID :many
diff --git a/internal/controlplane/handlers_artifacts.go b/internal/controlplane/handlers_artifacts.go
index cf646b62..41301a6b 100644
--- a/internal/controlplane/handlers_artifacts.go
+++ b/internal/controlplane/handlers_artifacts.go
@@ -69,14 +69,18 @@ func (s *Server) GetArtifactByName(ctx context.Context, in *pb.GetArtifactByName
 	nameParts := strings.Split(in.Name, "/")
 	if len(nameParts) < 3 {
 		return nil, util.UserVisibleError(codes.InvalidArgument, "invalid artifact name user repoOwner/repoName/artifactName")
 	}
 
+	entityCtx := engine.EntityFromContext(ctx)
+	projectID := entityCtx.Project.ID
+
 	repo, err := s.store.GetRepositoryByRepoName(ctx, db.GetRepositoryByRepoNameParams{
 		Provider:  in.GetContext().GetProvider(),
 		RepoOwner: nameParts[0],
 		RepoName:  nameParts[1],
+		ProjectID: projectID,
 	})
 	if err != nil {
 		if errors.Is(err, sql.ErrNoRows) {
 			return nil, util.UserVisibleError(codes.NotFound, "repository not found")
 		}
diff --git a/internal/controlplane/handlers_repositories.go b/internal/controlplane/handlers_repositories.go
index d755e218..8d2f05aa 100644
--- a/internal/controlplane/handlers_repositories.go
+++ b/internal/controlplane/handlers_repositories.go
@@ -272,12 +272,16 @@ func (s *Server) GetRepositoryByName(ctx context.Context,
 	provider, err := getProviderFromRequestOrDefault(ctx, s.store, in, projectID)
 	if err != nil {
 		return nil, providerError(err)
 	}
 
-	repo, err := s.store.GetRepositoryByRepoName(ctx,
-		db.GetRepositoryByRepoNameParams{Provider: provider.Name, RepoOwner: fragments[0], RepoName: fragments[1]})
+	repo, err := s.store.GetRepositoryByRepoName(ctx, db.GetRepositoryByRepoNameParams{
+		Provider:  provider.Name,
+		RepoOwner: fragments[0],
+		RepoName:  fragments[1],
+		ProjectID: projectID,
+	})
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return nil, status.Errorf(codes.NotFound, "repository not found")
 	} else if err != nil {
 		return nil, err
@@ -350,12 +354,16 @@ func (s *Server) DeleteRepositoryByName(ctx context.Context,
 	provider, err := getProviderFromRequestOrDefault(ctx, s.store, in, projectID)
 	if err != nil {
 		return nil, providerError(err)
 	}
 
-	repo, err := s.store.GetRepositoryByRepoName(ctx,
-		db.GetRepositoryByRepoNameParams{Provider: provider.Name, RepoOwner: fragments[0], RepoName: fragments[1]})
+	repo, err := s.store.GetRepositoryByRepoName(ctx, db.GetRepositoryByRepoNameParams{
+		Provider:  provider.Name,
+		RepoOwner: fragments[0],
+		RepoName:  fragments[1],
+		ProjectID: projectID,
+	})
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return nil, status.Errorf(codes.NotFound, "repository not found")
 	} else if err != nil {
 		return nil, err
diff --git a/internal/db/repositories.sql.go b/internal/db/repositories.sql.go
index dfbb416a..84ed8aa3 100644
--- a/internal/db/repositories.sql.go
+++ b/internal/db/repositories.sql.go
@@ -186,21 +186,27 @@ func (q *Queries) GetRepositoryByRepoID(ctx context.Context, repoID int64) (Repo
 	)
 	return i, err
 }
 
 const getRepositoryByRepoName = `-- name: GetRepositoryByRepoName :one
-SELECT id, provider, project_id, repo_owner, repo_name, repo_id, is_private, is_fork, webhook_id, webhook_url, deploy_url, clone_url, created_at, updated_at, default_branch FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3
+SELECT id, provider, project_id, repo_owner, repo_name, repo_id, is_private, is_fork, webhook_id, webhook_url, deploy_url, clone_url, created_at, updated_at, default_branch FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3 AND project_id = $4
 `
 
 type GetRepositoryByRepoNameParams struct {
-	Provider  string `json:"provider"`
-	RepoOwner string `json:"repo_owner"`
-	RepoName  string `json:"repo_name"`
+	Provider  string    `json:"provider"`
+	RepoOwner string    `json:"repo_owner"`
+	RepoName  string    `json:"repo_name"`
+	ProjectID uuid.UUID `json:"project_id"`
 }
 
 func (q *Queries) GetRepositoryByRepoName(ctx context.Context, arg GetRepositoryByRepoNameParams) (Repository, error) {
-	row := q.db.QueryRowContext(ctx, getRepositoryByRepoName, arg.Provider, arg.RepoOwner, arg.RepoName)
+	row := q.db.QueryRowContext(ctx, getRepositoryByRepoName,
+		arg.Provider,
+		arg.RepoOwner,
+		arg.RepoName,
+		arg.ProjectID,
+	)
 	var i Repository
 	err := row.Scan(
 		&i.ID,
 		&i.Provider,
 		&i.ProjectID,
diff --git a/internal/db/repositories_test.go b/internal/db/repositories_test.go
index 24e62de0..0f10f84a 100644
--- a/internal/db/repositories_test.go
+++ b/internal/db/repositories_test.go
@@ -127,11 +127,15 @@ func TestGetRepositoryByRepoName(t *testing.T) {
 	project := createRandomProject(t, org.ID)
 	prov := createRandomProvider(t, project.ID)
 	repo1 := createRandomRepository(t, project.ID, prov.Name)
 
 	repo2, err := testQueries.GetRepositoryByRepoName(context.Background(), GetRepositoryByRepoNameParams{
-		Provider: repo1.Provider, RepoOwner: repo1.RepoOwner, RepoName: repo1.RepoName})
+		Provider:  repo1.Provider,
+		RepoOwner: repo1.RepoOwner,
+		RepoName:  repo1.RepoName,
+		ProjectID: project.ID,
+	})
 	require.NoError(t, err)
 	require.NotEmpty(t, repo2)
 
 	require.Equal(t, repo1.ID, repo2.ID)
 	require.Equal(t, repo1.Provider, repo2.Provider)

Vulnerable code: SELECT * FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3;
	repo, err := s.store.GetRepositoryByRepoName(ctx,
		db.GetRepositoryByRepoNameParams{Provider: provider.Name, RepoOwner: fragments[0], RepoName: fragments[1]})
	repo, err := s.store.GetRepositoryByRepoName(ctx,
		db.GetRepositoryByRepoNameParams{Provider: provider.Name, RepoOwner: fragments[0], RepoName: fragments[1]})
SELECT id, provider, project_id, repo_owner, repo_name, repo_id, is_private, is_fork, webhook_id, webhook_url, deploy_url, clone_url, created_at, updated_at, default_branch FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3
	Provider  string `json:"provider"`
	RepoOwner string `json:"repo_owner"`
	RepoName  string `json:"repo_name"`
	row := q.db.QueryRowContext(ctx, getRepositoryByRepoName, arg.Provider, arg.RepoOwner, arg.RepoName)
		Provider: repo1.Provider, RepoOwner: repo1.RepoOwner, RepoName: repo1.RepoName})
Fixed code: SELECT * FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3 AND project_id = $4;
	entityCtx := engine.EntityFromContext(ctx)
	projectID := entityCtx.Project.ID

		ProjectID: projectID,
	repo, err := s.store.GetRepositoryByRepoName(ctx, db.GetRepositoryByRepoNameParams{
		Provider:  provider.Name,
		RepoOwner: fragments[0],
		RepoName:  fragments[1],
		ProjectID: projectID,
	})
	repo, err := s.store.GetRepositoryByRepoName(ctx, db.GetRepositoryByRepoNameParams{
		Provider:  provider.Name,
		RepoOwner: fragments[0],
		RepoName:  fragments[1],
		ProjectID: projectID,
	})
SELECT id, provider, project_id, repo_owner, repo_name, repo_id, is_private, is_fork, webhook_id, webhook_url, deploy_url, clone_url, created_at, updated_at, default_branch FROM repositories WHERE provider = $1 AND repo_owner = $2 AND repo_name = $3 AND project_id = $4
	Provider  string    `json:"provider"`
	RepoOwner string    `json:"repo_owner"`
	RepoName  string    `json:"repo_name"`
	ProjectID uuid.UUID `json:"project_id"`
	row := q.db.QueryRowContext(ctx, getRepositoryByRepoName,
		arg.Provider,
		arg.RepoOwner,
		arg.RepoName,
		arg.ProjectID,
	)
		Provider:  repo1.Provider,
		RepoOwner: repo1.RepoOwner,
		RepoName:  repo1.RepoName,
		ProjectID: project.ID,
	})