fix(caldav): enforce URL project match in GetResourcesByList

Multiget REPORT requests would happily return tasks from projects
different from the one in the href, even though GetTasksByUIDs now
filters by access. Drop any returned task whose real project_id does
not match the project ID parsed from the href path segment.

Hardening for GHSA-48ch-p4gq-x46x.
This commit is contained in:
kolaente 2026-04-09 17:17:09 +02:00 committed by kolaente
parent 200b787c16
commit 879462d717
3 changed files with 108 additions and 4 deletions

View File

@ -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,
}

View File

@ -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")
})
}

View File

@ -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 := `<?xml version="1.0" encoding="utf-8" ?>
<C:calendar-multiget xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
<D:prop>
<D:getetag/>
<C:calendar-data/>
</D:prop>
<D:href>/dav/projects/38/uid-caldav-test.ics</D:href>
</C:calendar-multiget>`
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 := `<?xml version="1.0" encoding="utf-8" ?>
<C:calendar-multiget xmlns:D="DAV:" xmlns:C="urn:ietf:params:xml:ns:caldav">
<D:prop>
<D:getetag/>
<C:calendar-data/>
</D:prop>
<D:href>/dav/projects/36/uid-caldav-test.ics</D:href>
</C:calendar-multiget>`
// 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) {