FramesController::index() computed $mon_options (the caller's per-monitor
restriction from unviewableMonitorIds()/viewableMonitorIds()) but never
applied it to the find() conditions, so any authenticated user with
Events=View could enumerate Frame rows (Id, EventId, MonitorId, TimeStamp,
Delta, Score, Type) for monitors they are explicitly denied via
GET /api/frames.json. Every sibling controller (EventsController,
ZonesController) and every other action in this same controller
(view/edit/delete, via eventForFrame()/requireFrameEdit()) already enforce
this restriction; index() was the one path left over from before the
per-monitor ACL helpers were added.
The naive fix of merging Event.MonitorId into $conditions the way
EventsController does does not work here: Frame belongsTo Event via
EventId, index() sets $this->Frame->recursive = -1, and Frame's own table
has no MonitorId column, so the condition can't resolve without a join.
Add an explicit inner join to Events (aliased Event) on
Event.Id = Frame.EventId whenever the caller has a monitor restriction,
and filter on Event.MonitorId, mirroring the explicit-join pattern
EventsController already uses for Tags.
Verified against the production database via a temporary CLI script
exercising the exact query-building logic: unrestricted find() returns
all ~110.5M frames (matching the pre-fix behaviour), while restricting to
a single monitor returns only that monitor's frames (~2.9M, cross-checked
row by row against the owning Event's MonitorId), and restricting to a
monitor with no events correctly returns zero. Reported as
GHSA-mg2g-jmfc-3w8g.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HoNiNxwgyaHV29CbiueCUf
Several API endpoints checked only the coarse Events/Monitors permission
and not the per-monitor object ACL, so a user explicitly denied a monitor
could still reach that monitor's objects by addressing them directly:
- EventsController::edit() and ::delete() checked Events=Edit but never
called canEdit() on the event, so any event could be mutated or deleted
by Id.
- FramesController only guaranteed Events != None in beforeFilter().
view() returned any frame by Id, and edit()/delete() mutated frames
without requiring Events=Edit or checking the parent event at all.
- ZonesController::forMonitor() listed zones for any monitor Id.
Resolve the owning object and apply the same canView()/canEdit() checks
the normal read paths already use. Frames are addressed by their own Id,
so their parent event is looked up to reach the monitor ACL.
Refs GHSA-hw39-qpjw-p7cg.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
