Skip to content

Fix IDOR in Kits, BroadcastAnnouncements, and Distributions controllers#5514

Open
costajohnt wants to merge 2 commits intorubyforgood:mainfrom
costajohnt:fix/5509-idor-kits-announcements-distributions
Open

Fix IDOR in Kits, BroadcastAnnouncements, and Distributions controllers#5514
costajohnt wants to merge 2 commits intorubyforgood:mainfrom
costajohnt:fix/5509-idor-kits-announcements-distributions

Conversation

@costajohnt
Copy link

Summary

Fixes cross-organization IDOR vulnerabilities (CWE-639) in three controllers by scoping all find(params[:id]) calls through current_organization:

  • KitsControllerdeactivate, reactivate, allocations, allocate (4 endpoints)
  • BroadcastAnnouncementsControllerset_broadcast_announcement before_action used by edit, update, destroy (3 endpoints)
  • DistributionsControllerprint, show, edit, update, destroy (5 endpoints)

Previously, these endpoints used unscoped Model.find(params[:id]), allowing an authenticated user from one organization to access, modify, or delete records belonging to a different organization by guessing or enumerating IDs.

Changes

  • Replaced Kit.find(params[:id]) with current_organization.kits.find(params[:id]) in 4 actions
  • Replaced BroadcastAnnouncement.find(params[:id]) with BroadcastAnnouncement.where(organization_id: current_organization.id).find(params[:id]) (no has_many association available)
  • Replaced Distribution.find(params[:id]) / Distribution.includes(...).find(params[:id]) with current_organization.distributions.find(params[:id]) in 5 actions
  • Added organization-scoped guard in destroy before delegating to DistributionDestroyService

Test Plan

  • Added cross-organization request specs for all fixed endpoints:
    • KitsController: deactivate, reactivate, allocations, allocate (4 tests)
    • BroadcastAnnouncementsController: edit, update, destroy (3 tests)
    • DistributionsController: print, show, edit, update, destroy (5 tests)
  • Each test creates a resource in a different organization and verifies a 404 response
  • For destructive actions (update, destroy), tests also verify the record is unchanged

Partial fix for #5509 (3 of 5 controllers).

…utions controllers

Scope all find-by-id queries through current_organization to prevent
cross-organization data access (CWE-639).

KitsController: deactivate, reactivate, allocations, allocate
BroadcastAnnouncementsController: set_broadcast_announcement (edit/update/destroy)
DistributionsController: print, show, edit, update, destroy

Closes rubyforgood#5509 (partial - 3 of 5 controllers)
@costajohnt costajohnt force-pushed the fix/5509-idor-kits-announcements-distributions branch from 9203d1b to 91fecf4 Compare March 13, 2026 04:30
Verify the distribution record is unchanged after a rejected
cross-org update attempt, matching the pattern used in the
broadcast announcements spec.
@costajohnt costajohnt marked this pull request as ready for review March 13, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant