Skip to content

Commit 0dd9751

Browse files
trim21trim21
authored andcommitted
fix(security): prevent unauthorized access to draft release attachments (#13934)
The `GetReleaseAttachment` API endpoint (`GET /repos/{owner}/{repo}/releases/{id}/assets/{attachment_id}`) and the web attachment download route (`ServeAttachment`, `GET /attachments/{uuid}`) did not check whether the release is a draft. Users holding only repository **read** permission (including unauthenticated callers on public repositories) could enumerate release/attachment IDs and retrieve the metadata and the full contents of attachments belonging to draft releases that are otherwise hidden from them. `GetRelease` and `ListReleaseAttachments` already return 404 for draft releases when the caller lacks write permission on the releases unit (added in the 2026-06-10 security patches), but these two endpoints were missed. This is the same class of issue fixed by Gitea in [CVE-2026-27660](https://nvd.nist.gov/vuln/detail/CVE-2026-27660) and [GHSA-q9pg-jj6x-j9p6](GHSA-q9pg-jj6x-j9p6). Co-authored-by: trim21 <i@trim21.me> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13934 Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
1 parent 1177bc1 commit 0dd9751

4 files changed

Lines changed: 79 additions & 24 deletions

File tree

release-notes/13934.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Users with only read permission on a repository (including unauthenticated callers on public repositories) could retrieve the metadata and the full contents of attachments belonging to draft releases, even though draft releases are hidden from them. The `GetReleaseAttachment` API endpoint and the web attachment download route (`ServeAttachment`) did not check whether the release is a draft, while `GetRelease` and `ListReleaseAttachments` already do. This is the same class of issue fixed upstream by Gitea in CVE-2026-27660 and GHSA-q9pg-jj6x-j9p6.

routers/api/v1/repo/release_attachment.go

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,27 @@ import (
2323
"forgejo.org/services/convert"
2424
)
2525

26-
func checkReleaseMatchRepo(ctx *context.APIContext, releaseID int64) bool {
26+
func checkReleaseAccess(ctx *context.APIContext, releaseID int64) *repo_model.Release {
2727
release, err := repo_model.GetReleaseByID(ctx, releaseID)
2828
if err != nil {
2929
if repo_model.IsErrReleaseNotExist(err) {
3030
ctx.NotFound()
31-
return false
31+
return nil
3232
}
3333
ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
34-
return false
34+
return nil
3535
}
3636
if release.RepoID != ctx.Repo().Repository.ID {
3737
ctx.NotFound()
38-
return false
38+
return nil
3939
}
40-
return true
40+
// Draft releases and their attachments must not be visible to users
41+
// without write permission on the releases unit, mirroring GetRelease.
42+
if release.IsDraft && !ctx.Repo().CanWrite(unit_model.TypeReleases) {
43+
ctx.NotFound()
44+
return nil
45+
}
46+
return release
4147
}
4248

4349
// GetReleaseAttachment gets a single attachment of the release
@@ -77,7 +83,7 @@ func GetReleaseAttachment(ctx *context.APIContext) {
7783
// "$ref": "#/responses/notFound"
7884

7985
releaseID := ctx.ParamsInt64(":id")
80-
if !checkReleaseMatchRepo(ctx, releaseID) {
86+
if checkReleaseAccess(ctx, releaseID) == nil {
8187
return
8288
}
8389

@@ -131,21 +137,8 @@ func ListReleaseAttachments(ctx *context.APIContext) {
131137
// "$ref": "#/responses/notFound"
132138

133139
releaseID := ctx.ParamsInt64(":id")
134-
release, err := repo_model.GetReleaseByID(ctx, releaseID)
135-
if err != nil {
136-
if repo_model.IsErrReleaseNotExist(err) {
137-
ctx.NotFound()
138-
return
139-
}
140-
ctx.Error(http.StatusInternalServerError, "GetReleaseByID", err)
141-
return
142-
}
143-
if release.RepoID != ctx.Repo().Repository.ID {
144-
ctx.NotFound()
145-
return
146-
}
147-
if release.IsDraft && !ctx.Repo().CanWrite(unit_model.TypeReleases) {
148-
ctx.NotFound()
140+
release := checkReleaseAccess(ctx, releaseID)
141+
if release == nil {
149142
return
150143
}
151144
if err := release.LoadAttributes(ctx); err != nil {
@@ -217,7 +210,7 @@ func CreateReleaseAttachment(ctx *context.APIContext) {
217210

218211
// Check if release exists an load release
219212
releaseID := ctx.ParamsInt64(":id")
220-
if !checkReleaseMatchRepo(ctx, releaseID) {
213+
if checkReleaseAccess(ctx, releaseID) == nil {
221214
return
222215
}
223216

@@ -362,7 +355,7 @@ func EditReleaseAttachment(ctx *context.APIContext) {
362355

363356
// Check if release exists an load release
364357
releaseID := ctx.ParamsInt64(":id")
365-
if !checkReleaseMatchRepo(ctx, releaseID) {
358+
if checkReleaseAccess(ctx, releaseID) == nil {
366359
return
367360
}
368361

@@ -443,7 +436,7 @@ func DeleteReleaseAttachment(ctx *context.APIContext) {
443436

444437
// Check if release exists an load release
445438
releaseID := ctx.ParamsInt64(":id")
446-
if !checkReleaseMatchRepo(ctx, releaseID) {
439+
if checkReleaseAccess(ctx, releaseID) == nil {
447440
return
448441
}
449442

routers/web/repo/attachment.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"forgejo.org/models/auth"
1111
access_model "forgejo.org/models/perm/access"
1212
repo_model "forgejo.org/models/repo"
13+
"forgejo.org/models/unit"
1314
"forgejo.org/modules/httpcache"
1415
"forgejo.org/modules/log"
1516
"forgejo.org/modules/setting"
@@ -153,6 +154,20 @@ func ServeAttachment(ctx *context.Context, uuid string) {
153154
ctx.Error(http.StatusNotFound)
154155
return
155156
}
157+
158+
// Attachments of draft releases must not be accessible without write
159+
// permission on the releases unit, mirroring the release API.
160+
if attach.ReleaseID != 0 {
161+
rel, err := repo_model.GetReleaseByID(ctx, attach.ReleaseID)
162+
if err != nil {
163+
ctx.ServerError("GetReleaseByID", err)
164+
return
165+
}
166+
if rel.IsDraft && !perm.CanWrite(unit.TypeReleases) {
167+
ctx.Error(http.StatusNotFound)
168+
return
169+
}
170+
}
156171
}
157172

158173
if attach.ExternalURL != "" {

tests/integration/api_releases_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,52 @@ func TestAPIReleaseGetAssets(t *testing.T) {
388388
})
389389
}
390390

391+
func TestAPIReleaseDraftAttachmentUnauthorizedAccess(t *testing.T) {
392+
defer tests.PrepareTestEnv(t)()
393+
394+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
395+
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
396+
ownerSession := loginUser(t, owner.LowerName)
397+
ownerToken := getTokenForLoggedInUser(t, ownerSession, auth_model.AccessTokenScopeWriteRepository)
398+
399+
rel := unittest.AssertExistsAndLoadBean(t, &repo_model.Release{
400+
RepoID: repo.ID,
401+
TagName: "draft-release",
402+
})
403+
assert.True(t, rel.IsDraft)
404+
405+
// sanity checks: the draft release itself and its attachment list are not
406+
// visible to anonymous users (only the repository read permission is held)
407+
req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d", owner.Name, repo.Name, rel.ID))
408+
MakeRequest(t, req, http.StatusNotFound)
409+
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", owner.Name, repo.Name, rel.ID))
410+
MakeRequest(t, req, http.StatusNotFound)
411+
412+
// the owner uploads an attachment to the draft release
413+
filename := "draft-secret.png"
414+
buff := generateImg()
415+
body := &bytes.Buffer{}
416+
contentType := tests.WriteImageBody(t, buff, filename, body)
417+
418+
assetURL := fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets", owner.Name, repo.Name, rel.ID)
419+
req = NewRequestWithBody(t, http.MethodPost, assetURL, bytes.NewReader(body.Bytes())).
420+
AddTokenAuth(ownerToken).
421+
SetHeader("Content-Type", contentType)
422+
resp := MakeRequest(t, req, http.StatusCreated)
423+
var attachment *api.Attachment
424+
DecodeJSON(t, resp, &attachment)
425+
require.NotNil(t, attachment)
426+
427+
// attachments of a draft release must not be accessible without write
428+
// permission on the releases unit: the attachment metadata ...
429+
req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/releases/%d/assets/%d", owner.Name, repo.Name, rel.ID, attachment.ID))
430+
MakeRequest(t, req, http.StatusNotFound)
431+
432+
// ... nor the attachment content
433+
req = NewRequest(t, "GET", "/attachments/"+attachment.UUID)
434+
MakeRequest(t, req, http.StatusNotFound)
435+
}
436+
391437
func TestAPIReleaseDeleteByTagName(t *testing.T) {
392438
defer tests.PrepareTestEnv(t)()
393439

0 commit comments

Comments
 (0)