Skip to content

Commit 57d01fe

Browse files
javokhir-secclaude
andauthored
fix: add project ownership check to getMilestone() to prevent IDOR (#3657)
The getMilestone() method had #[RequiresPermission] without projectIdParam and lacked a body-level project ownership check. Any authenticated user could read milestones from projects they don't belong to by guessing milestone IDs. Compare with getTicket() which properly verifies project assignment via isUserAssignedToProject(). CWE-639 (Authorization Bypass Through User-Controlled Key) CVSS:3.1 AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N (7.5) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5eee264 commit 57d01fe

1 file changed

Lines changed: 10 additions & 2 deletions

File tree

app/Domain/Tickets/Services/Tickets.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2910,10 +2910,18 @@ public function quickUpdateMilestone($params): array|bool
29102910
*
29112911
* @api
29122912
*/
2913-
#[RequiresPermission(TicketsPermissions::VIEW)]
2913+
#[RequiresPermission(TicketsPermissions::VIEW, projectIdParam: 'id')]
29142914
public function getMilestone(int $id): TicketModel|bool
29152915
{
2916-
return $this->ticketRepository->getTicket($id);
2916+
$milestone = $this->ticketRepository->getTicket($id);
2917+
2918+
// Verify the user is assigned to the milestone's project.
2919+
// Mirrors the authorization check in getTicket().
2920+
if ($milestone && $this->projectService->isUserAssignedToProject(session('userdata.id'), $milestone->projectId)) {
2921+
return $milestone;
2922+
}
2923+
2924+
return false;
29172925
}
29182926

29192927
/**

0 commit comments

Comments
 (0)