CakePHP's Model::set() takes the record id from a primary key in the
data passed to save(). edit() authorized the id in the URL and then saved
the request body, so Zone[Id]=<other> in the body wrote to that other
zone, past the per-monitor check just added. add() could likewise update
an existing row instead of creating one.
Add AppController::pinRequestId(), which drops the primary key from the
request data and sets the model id, and use it in these edits (pinned to
the URL id) and adds (cleared). Frames and EventData edit() never set the
model id at all, so a body without an Id inserted a new row rather than
updating; pinning fixes that too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FramesController::add() and EventDataController::add() saved request data
behind only the controllers' Events != None gate, so an Events=View user
could create rows for any event, including one on a monitor they are
denied. edit() checked the existing row but not the event or monitor the
request moved it to.
add() now requires Events=Edit and edit on the event named by EventId,
which covers that event's monitor. For EventData a supplied MonitorId must
also be viewable. edit() applies the same check to any EventId/MonitorId
in the request. Load includes/Event.php explicitly rather than relying on
the model association to have pulled it in.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
