Security: Cross-organization IDOR in 5 controllers (Requests, Donations, Kits, Announcements, Distributions)

[lighthousekeeper1212]

Summary

Multiple controllers use Model.find(params[:id]) without scoping to current_organization, allowing authenticated users from one organization to read and modify data belonging to other organizations.

Affected Controllers

1. RequestsController (state-changing)

• show (line 31): Request.find(params[:id]) — cross-org read
• start (line 45): Request.find(params[:id]) + request.status_started! — cross-org state change

Compare with index (line 6) which properly uses current_organization.ordered_requests.

2. DonationsController (data modification)

• print (line 6): Donation.find(params[:id]) — cross-org PDF generation
• show (line 66): Donation.find(params[:id]) — cross-org read
• edit (line 56): Donation.find(params[:id]) — cross-org read
• update (line 71): Donation.find(params[:id]) — cross-org modification

Compare with destroy (line 90) which properly uses current_organization.id.

3. KitsController (inventory manipulation)

• deactivate (line 47): Kit.find(params[:id]) — cross-org state change
• reactivate (line 53): Kit.find(params[:id]) — cross-org state change
• allocations (line 63): Kit.find(params[:id]) — cross-org read
• allocate (line 71): Kit.find(params[:id]) — cross-org inventory modification

Compare with index (line 7) which properly uses current_organization.kits.

4. BroadcastAnnouncementsController

• set_broadcast_announcement (line 48): BroadcastAnnouncement.find(params[:id]) — affects edit/update/destroy

Compare with index (line 5) which properly filters by organization_id: current_organization.id.

5. DistributionsController

• print (line 17): Distribution.find(params[:id]) — cross-org PDF with partner data

Root Cause

The authorize_user method in ApplicationController checks that the user has a valid role for some organization, but doesn't enforce that the accessed record belongs to the user's organization. Controllers that use Model.find(params[:id]) instead of current_organization.model_name.find(params[:id]) bypass organization isolation.

Fix

Replace Model.find(params[:id]) with current_organization.model_name.find(params[:id]) in all affected actions:

# Before (vulnerable)
@request = Request.find(params[:id])

# After (fixed)
@request = current_organization.requests.find(params[:id])

This pattern is already correctly used in many other controllers and some actions within the affected controllers themselves.

---

Found via automated security audit. CWE-639: Authorization Bypass Through User-Controlled Key.

[dorner]

This is a bot, but the findings seem to be valid. I'm not super fussed about the security issues here, but if someone wants to fix these, go for it 😁

[cassxw]

Hi! I can take this one on

[awwaiid]

@cassxw feel free to submit this as multiple independent small pull requests if you want -- that would possibly make it easier to verify (though the pattern here might be straightforward that a single PR would be fine? Not sure)

[costajohnt]

Hi! Following up on @awwaiid's suggestion to split this into multiple small PRs — I'd like to take on a few of these controllers.

@cassxw are you already working on any specific controllers? Happy to coordinate so we don't duplicate effort.

I can start with the KitsController and BroadcastAnnouncementsController fixes, and pick up DistributionsController as well if that works for everyone. The pattern is straightforward — scoping Model.find(params[:id]) to current_organization.

Draft PR: #5514

[dorner]

Hey @costajohnt! I appreciate the effort, but @cassxw did ask for the opportunity to work on this. I'd like to give them first dibs if that's OK. They can coordinate with you if they're OK with you helping out.

[cassxw]

Happy to coordinate, @costajohnt!

Created a PR for the fix for the remaining controllers (RequestsController and DonationsController) in #5519.