Fix comment permissions (#28213)

This PR will fix some missed checks for private repositories' data on
web routes and API routes.

13 files changed, 159 insertions, 30 deletions

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index 2a41619c3c..623c798fee 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -1259,8 +1259,8 @@ func Routes() *web.Route {
 			m.Group("/{username}/{reponame}", func() {
 				m.Group("/issues", func() {
 					m.Combo("").Get(repo.ListIssues).
-						Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), repo.CreateIssue)
-					m.Get("/pinned", repo.ListPinnedIssues)
+						Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueOption{}), reqRepoReader(unit.TypeIssues), repo.CreateIssue)
+					m.Get("/pinned", reqRepoReader(unit.TypeIssues), repo.ListPinnedIssues)
 					m.Group("/comments", func() {
 						m.Get("", repo.ListRepoIssueComments)
 						m.Group("/{id}", func() {
diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index 74e6361f6c..0f76a4b4ff 100644
--- a/routers/api/v1/repo/issue.go
+++ b/routers/api/v1/repo/issue.go
@@ -462,6 +462,24 @@ func ListIssues(ctx *context.APIContext) {
 		isPull = util.OptionalBoolNone
 	}
 
+	if isPull != util.OptionalBoolNone && !ctx.Repo.CanReadIssuesOrPulls(isPull.IsTrue()) {
+		ctx.NotFound()
+		return
+	}
+
+	if isPull == util.OptionalBoolNone {
+		canReadIssues := ctx.Repo.CanRead(unit.TypeIssues)
+		canReadPulls := ctx.Repo.CanRead(unit.TypePullRequests)
+		if !canReadIssues && !canReadPulls {
+			ctx.NotFound()
+			return
+		} else if !canReadIssues {
+			isPull = util.OptionalBoolTrue
+		} else if !canReadPulls {
+			isPull = util.OptionalBoolFalse
+		}
+	}
+
 	// FIXME: we should be more efficient here
 	createdByID := getUserIDForFilter(ctx, "created_by")
 	if ctx.Written() {
@@ -593,6 +611,10 @@ func GetIssue(ctx *context.APIContext) {
 		}
 		return
 	}
+	if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
+		ctx.NotFound()
+		return
+	}
 	ctx.JSON(http.StatusOK, convert.ToAPIIssue(ctx, issue))
 }
 
diff --git a/routers/api/v1/repo/issue_comment.go b/routers/api/v1/repo/issue_comment.go
index c718424f7e..4db2c68a79 100644
--- a/routers/api/v1/repo/issue_comment.go
+++ b/routers/api/v1/repo/issue_comment.go
@@ -12,9 +12,11 @@ import (
 	issues_model "code.gitea.io/gitea/models/issues"
 	access_model "code.gitea.io/gitea/models/perm/access"
 	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unit"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/context"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/api/v1/utils"
 	"code.gitea.io/gitea/services/convert"
@@ -71,6 +73,11 @@ func ListIssueComments(ctx *context.APIContext) {
 		ctx.Error(http.StatusInternalServerError, "GetRawIssueByIndex", err)
 		return
 	}
+	if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
+		ctx.NotFound()
+		return
+	}
+
 	issue.Repo = ctx.Repo.Repository
 
 	opts := &issues_model.FindCommentsOptions{
@@ -271,12 +278,27 @@ func ListRepoIssueComments(ctx *context.APIContext) {
 		return
 	}
 
+	var isPull util.OptionalBool
+	canReadIssue := ctx.Repo.CanRead(unit.TypeIssues)
+	canReadPull := ctx.Repo.CanRead(unit.TypePullRequests)
+	if canReadIssue && canReadPull {
+		isPull = util.OptionalBoolNone
+	} else if canReadIssue {
+		isPull = util.OptionalBoolFalse
+	} else if canReadPull {
+		isPull = util.OptionalBoolTrue
+	} else {
+		ctx.NotFound()
+		return
+	}
+
 	opts := &issues_model.FindCommentsOptions{
 		ListOptions: utils.GetListOptions(ctx),
 		RepoID:      ctx.Repo.Repository.ID,
 		Type:        issues_model.CommentTypeComment,
 		Since:       since,
 		Before:      before,
+		IsPull:      isPull,
 	}
 
 	comments, err := issues_model.FindComments(ctx, opts)
@@ -367,6 +389,11 @@ func CreateIssueComment(ctx *context.APIContext) {
 		return
 	}
 
+	if !ctx.Repo.CanReadIssuesOrPulls(issue.IsPull) {
+		ctx.NotFound()
+		return
+	}
+
 	if issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(issue.IsPull) && !ctx.Doer.IsAdmin {
 		ctx.Error(http.StatusForbidden, "CreateIssueComment", errors.New(ctx.Tr("repo.issues.comment_on_locked")))
 		return
@@ -436,6 +463,11 @@ func GetIssueComment(ctx *context.APIContext) {
 		return
 	}
 
+	if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+		ctx.NotFound()
+		return
+	}
+
 	if comment.Type != issues_model.CommentTypeComment {
 		ctx.Status(http.StatusNoContent)
 		return
@@ -555,7 +587,17 @@ func editIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption)
 		return
 	}
 
-	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.IsAdmin()) {
+	if err := comment.LoadIssue(ctx); err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
+		return
+	}
+
+	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+		ctx.Status(http.StatusNotFound)
+		return
+	}
+
+	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
 		ctx.Status(http.StatusForbidden)
 		return
 	}
@@ -658,7 +700,17 @@ func deleteIssueComment(ctx *context.APIContext) {
 		return
 	}
 
-	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.IsAdmin()) {
+	if err := comment.LoadIssue(ctx); err != nil {
+		ctx.Error(http.StatusInternalServerError, "LoadIssue", err)
+		return
+	}
+
+	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+		ctx.Status(http.StatusNotFound)
+		return
+	}
+
+	if !ctx.IsSigned || (ctx.Doer.ID != comment.PosterID && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull)) {
 		ctx.Status(http.StatusForbidden)
 		return
 	} else if comment.Type != issues_model.CommentTypeComment {
diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go
index c327c54d10..21e2f4dabd 100644
--- a/routers/api/v1/repo/issue_comment_attachment.go
+++ b/routers/api/v1/repo/issue_comment_attachment.go
@@ -329,6 +329,10 @@ func getIssueCommentSafe(ctx *context.APIContext) *issues_model.Comment {
 		return nil
 	}
 
+	if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+		return nil
+	}
+
 	comment.Issue.Repo = ctx.Repo.Repository
 
 	return comment
diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go
index 29c99184e7..c886bd71b7 100644
--- a/routers/api/v1/repo/issue_reaction.go
+++ b/routers/api/v1/repo/issue_reaction.go
@@ -61,6 +61,12 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 
 	if err := comment.LoadIssue(ctx); err != nil {
 		ctx.Error(http.StatusInternalServerError, "comment.LoadIssue", err)
+		return
+	}
+
+	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+		ctx.NotFound()
+		return
 	}
 
 	if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
@@ -190,9 +196,19 @@ func changeIssueCommentReaction(ctx *context.APIContext, form api.EditReactionOp
 		return
 	}
 
-	err = comment.LoadIssue(ctx)
-	if err != nil {
+	if err = comment.LoadIssue(ctx); err != nil {
 		ctx.Error(http.StatusInternalServerError, "comment.LoadIssue() failed", err)
+		return
+	}
+
+	if comment.Issue.RepoID != ctx.Repo.Repository.ID {
+		ctx.NotFound()
+		return
+	}
+
+	if !ctx.Repo.CanReadIssuesOrPulls(comment.Issue.IsPull) {
+		ctx.NotFound()
+		return
 	}
 
 	if comment.Issue.IsLocked && !ctx.Repo.CanWriteIssuesOrPulls(comment.Issue.IsPull) {
diff --git a/routers/api/v1/repo/key.go b/routers/api/v1/repo/key.go
index 3dc5a60d1c..af48c40885 100644
--- a/routers/api/v1/repo/key.go
+++ b/routers/api/v1/repo/key.go
@@ -153,6 +153,12 @@ func GetDeployKey(ctx *context.APIContext) {
 		return
 	}
 
+	// this check make it more consistent
+	if key.RepoID != ctx.Repo.Repository.ID {
+		ctx.NotFound()
+		return
+	}
+
 	if err = key.GetContent(ctx); err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetContent", err)
 		return
diff --git a/routers/api/v1/repo/release.go b/routers/api/v1/repo/release.go
index 61e5bdd679..6c70bffca3 100644
--- a/routers/api/v1/repo/release.go
+++ b/routers/api/v1/repo/release.go
@@ -49,13 +49,12 @@ func GetRelease(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	id := ctx.ParamsInt64(":id")
-	release, err := repo_model.GetReleaseByID(ctx, id)
+	release, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
 	if err != nil && !repo_model.IsErrReleaseNotExist(err) {
-		ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+		ctx.Error(http.StatusInternalServerError, "GetReleaseForRepoByID", err)
 		return
 	}
-	if err != nil && repo_model.IsErrReleaseNotExist(err) ||
-		release.IsTag || release.RepoID != ctx.Repo.Repository.ID {
+	if err != nil && repo_model.IsErrReleaseNotExist(err) || release.IsTag {
 		ctx.NotFound()
 		return
 	}
@@ -315,13 +314,12 @@ func EditRelease(ctx *context.APIContext) {
 
 	form := web.GetForm(ctx).(*api.EditReleaseOption)
 	id := ctx.ParamsInt64(":id")
-	rel, err := repo_model.GetReleaseByID(ctx, id)
+	rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
 	if err != nil && !repo_model.IsErrReleaseNotExist(err) {
-		ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+		ctx.Error(http.StatusInternalServerError, "GetReleaseForRepoByID", err)
 		return
 	}
-	if err != nil && repo_model.IsErrReleaseNotExist(err) ||
-		rel.IsTag || rel.RepoID != ctx.Repo.Repository.ID {
+	if err != nil && repo_model.IsErrReleaseNotExist(err) || rel.IsTag {
 		ctx.NotFound()
 		return
 	}
@@ -393,17 +391,16 @@ func DeleteRelease(ctx *context.APIContext) {
 	//     "$ref": "#/responses/empty"
 
 	id := ctx.ParamsInt64(":id")
-	rel, err := repo_model.GetReleaseByID(ctx, id)
+	rel, err := repo_model.GetReleaseForRepoByID(ctx, ctx.Repo.Repository.ID, id)
 	if err != nil && !repo_model.IsErrReleaseNotExist(err) {
-		ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+		ctx.Error(http.StatusInternalServerError, "GetReleaseForRepoByID", err)
 		return
 	}
-	if err != nil && repo_model.IsErrReleaseNotExist(err) ||
-		rel.IsTag || rel.RepoID != ctx.Repo.Repository.ID {
+	if err != nil && repo_model.IsErrReleaseNotExist(err) || rel.IsTag {
 		ctx.NotFound()
 		return
 	}
-	if err := release_service.DeleteReleaseByID(ctx, id, ctx.Doer, false); err != nil {
+	if err := release_service.DeleteReleaseByID(ctx, ctx.Repo.Repository, rel, ctx.Doer, false); err != nil {
 		if models.IsErrProtectedTagName(err) {
 			ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
 			return
diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go
index 168ef550c5..c36bf12e6d 100644
--- a/routers/api/v1/repo/release_attachment.go
+++ b/routers/api/v1/repo/release_attachment.go
@@ -17,6 +17,23 @@ import (
 	"code.gitea.io/gitea/services/convert"
 )
 
+func checkReleaseMatchRepo(ctx *context.APIContext, releaseID int64) bool {
+	release, err := repo_model.GetReleaseByID(ctx, releaseID)
+	if err != nil {
+		if repo_model.IsErrReleaseNotExist(err) {
+			ctx.NotFound()
+			return false
+		}
+		ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+		return false
+	}
+	if release.RepoID != ctx.Repo.Repository.ID {
+		ctx.NotFound()
+		return false
+	}
+	return true
+}
+
 // GetReleaseAttachment gets a single attachment of the release
 func GetReleaseAttachment(ctx *context.APIContext) {
 	// swagger:operation GET /repos/{owner}/{repo}/releases/{id}/assets/{attachment_id} repository repoGetReleaseAttachment
@@ -54,6 +71,10 @@ func GetReleaseAttachment(ctx *context.APIContext) {
 	//     "$ref": "#/responses/notFound"
 
 	releaseID := ctx.ParamsInt64(":id")
+	if !checkReleaseMatchRepo(ctx, releaseID) {
+		return
+	}
+
 	attachID := ctx.ParamsInt64(":attachment_id")
 	attach, err := repo_model.GetAttachmentByID(ctx, attachID)
 	if err != nil {
@@ -176,13 +197,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 
 	// Check if release exists an load release
 	releaseID := ctx.ParamsInt64(":id")
-	release, err := repo_model.GetReleaseByID(ctx, releaseID)
-	if err != nil {
-		if repo_model.IsErrReleaseNotExist(err) {
-			ctx.NotFound()
-			return
-		}
-		ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
+	if !checkReleaseMatchRepo(ctx, releaseID) {
 		return
 	}
 
@@ -203,7 +218,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
 	attach, err := attachment.UploadAttachment(ctx, file, setting.Repository.Release.AllowedTypes, header.Size, &repo_model.Attachment{
 		Name:       filename,
 		UploaderID: ctx.Doer.ID,
-		RepoID:     release.RepoID,
+		RepoID:     ctx.Repo.Repository.ID,
 		ReleaseID:  releaseID,
 	})
 	if err != nil {
@@ -264,6 +279,10 @@ func EditReleaseAttachment(ctx *context.APIContext) {
 
 	// Check if release exists an load release
 	releaseID := ctx.ParamsInt64(":id")
+	if !checkReleaseMatchRepo(ctx, releaseID) {
+		return
+	}
+
 	attachID := ctx.ParamsInt64(":attachment_id")
 	attach, err := repo_model.GetAttachmentByID(ctx, attachID)
 	if err != nil {
@@ -328,6 +347,10 @@ func DeleteReleaseAttachment(ctx *context.APIContext) {
 
 	// Check if release exists an load release
 	releaseID := ctx.ParamsInt64(":id")
+	if !checkReleaseMatchRepo(ctx, releaseID) {
+		return
+	}
+
 	attachID := ctx.ParamsInt64(":attachment_id")
 	attach, err := repo_model.GetAttachmentByID(ctx, attachID)
 	if err != nil {
diff --git a/routers/api/v1/repo/release_tags.go b/routers/api/v1/repo/release_tags.go
index 926a713c94..9f2098df06 100644
--- a/routers/api/v1/repo/release_tags.go
+++ b/routers/api/v1/repo/release_tags.go
@@ -112,7 +112,7 @@ func DeleteReleaseByTag(ctx *context.APIContext) {
 		return
 	}
 
-	if err = releaseservice.DeleteReleaseByID(ctx, release.ID, ctx.Doer, false); err != nil {
+	if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, release, ctx.Doer, false); err != nil {
 		if models.IsErrProtectedTagName(err) {
 			ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
 			return
diff --git a/routers/api/v1/repo/tag.go b/routers/api/v1/repo/tag.go
index dbc8df0ef8..2f19f95e66 100644
--- a/routers/api/v1/repo/tag.go
+++ b/routers/api/v1/repo/tag.go
@@ -272,7 +272,7 @@ func DeleteTag(ctx *context.APIContext) {
 		return
 	}
 
-	if err = releaseservice.DeleteReleaseByID(ctx, tag.ID, ctx.Doer, true); err != nil {
+	if err = releaseservice.DeleteReleaseByID(ctx, ctx.Repo.Repository, tag, ctx.Doer, true); err != nil {
 		if models.IsErrProtectedTagName(err) {
 			ctx.Error(http.StatusMethodNotAllowed, "delTag", "user not allowed to delete protected tag")
 			return
diff --git a/routers/api/v1/user/app.go b/routers/api/v1/user/app.go
index edbc1d17b4..f045fb4d5d 100644
--- a/routers/api/v1/user/app.go
+++ b/routers/api/v1/user/app.go
@@ -342,6 +342,10 @@ func GetOauth2Application(ctx *context.APIContext) {
 		}
 		return
 	}
+	if app.UID != ctx.Doer.ID {
+		ctx.NotFound()
+		return
+	}
 
 	app.ClientSecret = ""
 
diff --git a/routers/api/v1/user/gpg_key.go b/routers/api/v1/user/gpg_key.go
index 404b1d221e..4f8bcaca3e 100644
--- a/routers/api/v1/user/gpg_key.go
+++ b/routers/api/v1/user/gpg_key.go
@@ -112,7 +112,7 @@ func GetGPGKey(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	key, err := asymkey_model.GetGPGKeyByID(ctx, ctx.ParamsInt64(":id"))
+	key, err := asymkey_model.GetGPGKeyForUserByID(ctx, ctx.Doer.ID, ctx.ParamsInt64(":id"))
 	if err != nil {
 		if asymkey_model.IsErrGPGKeyNotExist(err) {
 			ctx.NotFound()
diff --git a/routers/api/v1/user/hook.go b/routers/api/v1/user/hook.go
index 50be519c81..e87385e4a2 100644
--- a/routers/api/v1/user/hook.go
+++ b/routers/api/v1/user/hook.go
@@ -62,6 +62,11 @@ func GetHook(ctx *context.APIContext) {
 		return
 	}
 
+	if !ctx.Doer.IsAdmin && hook.OwnerID != ctx.Doer.ID {
+		ctx.NotFound()
+		return
+	}
+
 	apiHook, err := webhook_service.ToHook(ctx.Doer.HomeLink(), hook)
 	if err != nil {
 		ctx.InternalServerError(err)