Threats Tagged 'ghsa-9rg3-v78m-26q8'

## Summary API token permission checks only match the HTTP method and route path. The `expand` query parameter on task read endpoints embeds data from other permission groups (task comments, reactions, time entry counts) without checking whether the token holds those scopes. A token scoped only to `tasks` read permissions can therefore read task comments and reactions it was explicitly not granted. ## Details `models.CanDoAPIRoute` (`pkg/models/api_routes.go`, ~line 440) authorises API tokens purely by comparing `method` and `c.Path()` against the routes stored for each granted permission group. The query string is never consulted. The task read endpoints accept `expand`: - `GET /api/v1/tasks/:task`, `GET /api/v1/tasks` (`pkg/models/tasks.go` ReadOne/ReadAll, `pkg/models/task_collection.go`) - `GET /api/v2/tasks/:id`, `GET /api/v2/tasks`, `GET /api/v2/projects/:project/tasks` (`pkg/routes/api/v2/tasks.go`, `task_collection.go`) Accepted values include `comments`, `comment_count`, `reactions`, `time_entries_count`. `addMoreInfoToTasks` (`pkg/models/tasks.go`, ~line 683) loads and embeds that data. At that layer only a `web.Auth` (the plain owner user, resolved by `auth.GetAuthFromClaims`) is available, so the token's scopes cannot be enforced there either. Result: the `tasks_comments`, `reactions`, and `time_entries` permission groups are advisory for any data reachable through a task expansion. ## Impact A holder of an API token scoped to `tasks: [read_one]` or `tasks: [read_all]` (or `projects_views_tasks: [read_all]`) can read the full bodies of task comments, all reactions, and time entry counts on every task the token owner can access, despite `GET /api/v1|v2/tasks/:task/comments` and the reactions endpoints correctly returning 401 for the same token. The leak is limited to data the token owner can already see, and is read-only. The realistic victim is a user who grants a narrowly scoped token to a third-party integration expecting comments to stay private. ## Proof of Concept Against the test fixtures (`pkg/db/fixtures/api_tokens.yml`, token 1 has `{"tasks":["read_all","update"]}`, plaintext `tk_2eef46f40ebab3304919ab2e7e39993f75f29d2e`): ``` GET /api/v2/tasks/1/comments Authorization: Bearer tk_2eef46f40ebab3304919ab2e7e39993f75f29d2e -> 401 {"code":11,"message":"missing, malformed, expired or otherwise invalid token provided"} GET /api/v2/tasks?expand=comments&filter=id%3D1 Authorization: Bearer tk_2eef46f40ebab3304919ab2e7e39993f75f29d2e -> 200, response items[0].comments contains the full comment objects ``` Same behaviour with `expand=reactions`, and on the v1 endpoints `GET /api/v1/tasks?expand=comments` / `GET /api/v1/tasks/1?expand=comments`. Any user-created token with only `tasks` read permissions reproduces this. ## Recommended Fix Enforce expansions in the single existing choke point, `models.CanDoAPIRoute`: after the method/path match succeeds, read `c.QueryParams()["expand"]` and require the token to hold the owning group's read permission for each value, e.g. - `comments`, `comment_count` -> `tasks_comments.read_all` - `reactions` -> `reactions.read_all` - `time_entries_count` -> `time_entries.read_all` (`subtasks`, `buckets`, `is_unread` are task-level data and need nothing extra.) Doing this in the middleware covers both v1 and v2 without handler or model changes. Add a table-driven test alongside `pkg/webtests/api_token_method_matching_test.go` asserting a `tasks`-only token gets 401 with `expand=comments` and 200 once `tasks_comments.read_all` is added.