From 9efe1fadba817923c7c7f5953c3e9e9c5683bbf3 Mon Sep 17 00:00:00 2001 From: kolaente Date: Mon, 23 Mar 2026 16:07:37 +0100 Subject: [PATCH] fix: block link share users from listing link shares in ReadAll Link share authenticated users could call ReadAll on link shares, which leaked hash credentials for other shares on the same project. This allowed permission escalation from read-only to write/admin. Add a check at the top of ReadAll() that rejects link-share-authenticated callers, mirroring the pattern in CanRead() and canDoLinkShare(). Update tests to expect 403 Forbidden for all link share permission levels. Fixes GHSA-8hp8-9fhr-pfm9 --- pkg/models/link_sharing.go | 5 +++++ pkg/webtests/link_sharing_test.go | 18 +++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go index 9c3124f43..99bf7d118 100644 --- a/pkg/models/link_sharing.go +++ b/pkg/models/link_sharing.go @@ -230,6 +230,11 @@ func (share *LinkSharing) ReadOne(s *xorm.Session, _ web.Auth) (err error) { // @Failure 500 {object} models.Message "Internal error" // @Router /projects/{project}/shares [get] func (share *LinkSharing) ReadAll(s *xorm.Session, a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) { + // Don't allow link share authenticated users to list link shares + if _, is := a.(*LinkSharing); is { + return nil, 0, 0, ErrGenericForbidden{} + } + project := &Project{ID: share.ProjectID} can, _, err := project.CanRead(s, a) if err != nil { diff --git a/pkg/webtests/link_sharing_test.go b/pkg/webtests/link_sharing_test.go index a4ddc527e..ec2f55219 100644 --- a/pkg/webtests/link_sharing_test.go +++ b/pkg/webtests/link_sharing_test.go @@ -739,19 +739,19 @@ func TestLinkSharing(t *testing.T) { } t.Run("ReadAll", func(t *testing.T) { t.Run("Shared readonly", func(t *testing.T) { - rec, err := testHandlerLinkShareReadOnly.testReadAllWithLinkShare(nil, map[string]string{"project": "1"}) - require.NoError(t, err) - assert.Contains(t, rec.Body.String(), `"hash":"test"`) + _, err := testHandlerLinkShareReadOnly.testReadAllWithLinkShare(nil, map[string]string{"project": "1"}) + require.Error(t, err) + assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`) }) t.Run("Shared write", func(t *testing.T) { - rec, err := testHandlerLinkShareWrite.testReadAllWithLinkShare(nil, map[string]string{"project": "2"}) - require.NoError(t, err) - assert.Contains(t, rec.Body.String(), `"hash":"test2"`) + _, err := testHandlerLinkShareWrite.testReadAllWithLinkShare(nil, map[string]string{"project": "2"}) + require.Error(t, err) + assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`) }) t.Run("Shared admin", func(t *testing.T) { - rec, err := testHandlerLinkShareAdmin.testReadAllWithLinkShare(nil, map[string]string{"project": "3"}) - require.NoError(t, err) - assert.Contains(t, rec.Body.String(), `"hash":"test3"`) + _, err := testHandlerLinkShareAdmin.testReadAllWithLinkShare(nil, map[string]string{"project": "3"}) + require.Error(t, err) + assert.Contains(t, getHTTPErrorMessage(err), `Forbidden`) }) }) t.Run("Create", func(t *testing.T) {