diff --git a/pkg/routes/caldav/listStorageProvider.go b/pkg/routes/caldav/listStorageProvider.go index ab0ccad31..5544d3ec7 100644 --- a/pkg/routes/caldav/listStorageProvider.go +++ b/pkg/routes/caldav/listStorageProvider.go @@ -175,16 +175,32 @@ func principalPathForUser(username string) string { // GetResourcesByList fetches a list of resources from a slice of paths func (vcls *VikunjaCaldavProjectStorage) GetResourcesByList(rpaths []string) (resources []data.Resource, err error) { - // Parse the set of resourcepaths into usable uids - // A path looks like this: /dav/projects/10/a6eb526d5748a5c499da202fe74f36ed1aea2aef.ics - // So we split the url in parts, take the last one and strip the ".ics" at the end + // Path format: /dav/projects/{projectID}/{uid}.ics. + // Remember the href's project ID per uid so the consistency check below + // can drop tasks requested via the wrong project (GHSA-48ch-p4gq-x46x). var uids []string + uidURLProjects := map[string]int64{} for _, path := range rpaths { parts := strings.Split(path, "/") if len(parts) < 5 { continue } - uids = append(uids, strings.TrimSuffix(parts[4], ".ics")) + // Skip malformed hrefs: without these guards an empty/non-numeric + // project would bypass the consistency check, and an empty UID + // would query for tasks with empty/NULL uids. + if !strings.HasSuffix(parts[4], ".ics") { + continue + } + uid := strings.TrimSuffix(parts[4], ".ics") + if uid == "" { + continue + } + urlProjectID, perr := strconv.ParseInt(parts[3], 10, 64) + if perr != nil { + continue + } + uids = append(uids, uid) + uidURLProjects[uid] = urlProjectID } if len(uids) == 0 { @@ -207,6 +223,11 @@ func (vcls *VikunjaCaldavProjectStorage) GetResourcesByList(rpaths []string) (re } for _, t := range tasks { + // Closes the URL-path leak for calendar-multiget REPORTs + // (GHSA-48ch-p4gq-x46x). + if urlProjectID, ok := uidURLProjects[t.UID]; ok && urlProjectID != t.ProjectID { + continue + } rr := VikunjaProjectResourceAdapter{ task: t, } diff --git a/pkg/routes/caldav/listStorageProvider_test.go b/pkg/routes/caldav/listStorageProvider_test.go index caa92fec7..477b58e20 100644 --- a/pkg/routes/caldav/listStorageProvider_test.go +++ b/pkg/routes/caldav/listStorageProvider_test.go @@ -486,3 +486,50 @@ END:VCALENDAR` assert.Equal(t, "uid-caldav-test-child-task-2", formerSiblingSubTask.UID) }) } + +// TestGetResourcesByList_URLProjectConsistency covers GHSA-48ch-p4gq-x46x. +func TestGetResourcesByList_URLProjectConsistency(t *testing.T) { + u := &user.User{ + ID: 15, + Username: "user15", + } + + t.Run("drops task when href project does not match task project", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + + storage := &VikunjaCaldavProjectStorage{user: u} + + // uid-caldav-test lives in project 36 (fixtures/tasks.yml id 40). + resources, err := storage.GetResourcesByList([]string{ + "/dav/projects/38/uid-caldav-test.ics", + }) + require.NoError(t, err) + assert.Empty(t, resources, "task must not be returned when href project mismatches") + }) + + t.Run("returns task when href project matches task project", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + + storage := &VikunjaCaldavProjectStorage{user: u} + + resources, err := storage.GetResourcesByList([]string{ + "/dav/projects/36/uid-caldav-test.ics", + }) + require.NoError(t, err) + require.Len(t, resources, 1) + }) + + t.Run("drops task for unauthorized user", func(t *testing.T) { + db.LoadAndAssertFixtures(t) + + // user 6 has no access to project 36 + outsider := &user.User{ID: 6, Username: "user6"} + storage := &VikunjaCaldavProjectStorage{user: outsider} + + resources, err := storage.GetResourcesByList([]string{ + "/dav/projects/36/uid-caldav-test.ics", + }) + require.NoError(t, err) + assert.Empty(t, resources, "unauthorized user must not receive any tasks") + }) +} diff --git a/pkg/webtests/caldav_test.go b/pkg/webtests/caldav_test.go index f338db71f..900772d0c 100644 --- a/pkg/webtests/caldav_test.go +++ b/pkg/webtests/caldav_test.go @@ -107,6 +107,42 @@ END:VCALENDAR` assert.NotContains(t, rec.Body.String(), "Title Caldav Test") assert.Equal(t, http.StatusNotFound, rec.Result().StatusCode) }) + t.Run("Multiget rejects UIDs from other projects", func(t *testing.T) { + e, _ := setupTestEnv() + + // href claims project 38, but uid-caldav-test lives in project 36. + reportBody := ` + + + + + + /dav/projects/38/uid-caldav-test.ics +` + + rec, err := newCaldavTestRequestWithUser(t, e, "REPORT", caldav.ProjectHandler, &testuser15, reportBody, nil, map[string]string{"project": "38"}) + require.NoError(t, err) + assert.NotContains(t, rec.Body.String(), "Title Caldav Test") + assert.NotContains(t, rec.Body.String(), "Description Caldav Test") + }) + t.Run("Multiget cross-user UID read is rejected", func(t *testing.T) { + e, _ := setupTestEnv() + + reportBody := ` + + + + + + /dav/projects/36/uid-caldav-test.ics +` + + // testuser6 cannot access project 36 where uid-caldav-test lives. + rec, err := newCaldavTestRequestWithUser(t, e, "REPORT", caldav.ProjectHandler, &testuser6, reportBody, nil, map[string]string{"project": "36"}) + require.NoError(t, err) + assert.NotContains(t, rec.Body.String(), "Title Caldav Test") + assert.NotContains(t, rec.Body.String(), "Description Caldav Test") + }) } func TestCaldavDiscovery(t *testing.T) {