fix(attachments): enforce owning repository path (#39048) (#39077)

Backport #39048 by @bircni

Reject attachment requests routed through a repository other than the
attachment owner.

Co-authored-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

routers/web/repo/attachment.go

@@ -143,18 +143,18 @@ func ServeAttachment(ctx *context.Context, uuid string) {
 		return
 	}
 
-	// prevent visiting attachment from other repository directly
-	// The check will be ignored before this code merged.
-	if attach.CreatedUnix > repo_model.LegacyAttachmentMissingRepoIDCutoff && ctx.Repo.Repository != nil && ctx.Repo.Repository.ID != attach.RepoID {
-		ctx.HTTPError(http.StatusNotFound)
-		return
-	}
-
 	unitType, repoID, err := repo_service.GetAttachmentLinkedTypeAndRepoID(ctx, attach)
 	if err != nil {
 		ctx.ServerError("GetAttachmentLinkedTypeAndRepoID", err)
 		return
 	}
+	if repoID == 0 {
+		repoID = attach.RepoID
+	}
+	if ctx.Repo.Repository != nil && repoID != 0 && ctx.Repo.Repository.ID != repoID {
+		ctx.HTTPError(http.StatusNotFound)
+		return
+	}
 
 	if unitType == unit.TypeInvalid { // unlinked attachment can only be accessed by the uploader
 		if !(ctx.IsSigned && attach.UploaderID == ctx.Doer.ID) { // We block if not the uploader

tests/integration/attachment_test.go

@@ -189,6 +189,26 @@ func testGetAttachment(t *testing.T) {
 			tc.session.MakeRequest(t, req, tc.want)
 		})
 	}
+
+	attachment, err := repo_model.GetAttachmentByUUID(t.Context(), "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12")
+	require.NoError(t, err)
+	defer func() {
+		attachment.RepoID = 2
+		require.NoError(t, repo_model.UpdateAttachmentByUUID(t.Context(), attachment, "repo_id"))
+	}()
+	for _, testCase := range []struct {
+		name   string
+		repoID int64
+	}{
+		{"RecordedRepository", 2},
+		{"LegacyMissingRepository", 0},
+	} {
+		t.Run("OtherRepositoryPath/"+testCase.name, func(t *testing.T) {
+			attachment.RepoID = testCase.repoID
+			require.NoError(t, repo_model.UpdateAttachmentByUUID(t.Context(), attachment, "repo_id"))
+			MakeRequest(t, NewRequest(t, "GET", "/user2/repo1/attachments/"+attachment.UUID), http.StatusNotFound)
+		})
+	}
 }
 
 func testDeleteAttachmentPermissions(t *testing.T) {