Skip to content

Commit 516bab0

Browse files
fix: enforce per-monitor ACL on direct event, frame and zone API endpoints
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>
1 parent 6daa135 commit 516bab0

3 files changed

Lines changed: 77 additions & 0 deletions

File tree

web/api/app/Controller/EventsController.php

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,18 @@ public function edit($id = null) {
348348
throw new NotFoundException(__('Invalid event'));
349349
}
350350

351+
# Events=Edit is coarse. Enforce the per-monitor ACL too, otherwise a user
352+
# denied a monitor can still mutate that monitor's events by direct Id.
353+
$this->Event->recursive = -1;
354+
$event = $this->Event->find('first', array(
355+
'conditions' => array('Event.' . $this->Event->primaryKey => $id)
356+
));
357+
$EventObj = new ZM\Event($event['Event']);
358+
if ( !$EventObj->canEdit() ) {
359+
throw new UnauthorizedException(__('Insufficient Privileges'));
360+
return;
361+
}
362+
351363
if ( $this->Event->save($this->request->data) ) {
352364
$message = 'Saved';
353365
} else {
@@ -379,6 +391,19 @@ public function delete($id = null) {
379391
throw new NotFoundException(__('Invalid event'));
380392
}
381393
$this->request->allowMethod('post', 'delete');
394+
395+
# Events=Edit is coarse. Enforce the per-monitor ACL too, otherwise a user
396+
# denied a monitor can still delete that monitor's events by direct Id.
397+
$this->Event->recursive = -1;
398+
$event = $this->Event->find('first', array(
399+
'conditions' => array('Event.' . $this->Event->primaryKey => $id)
400+
));
401+
$EventObj = new ZM\Event($event['Event']);
402+
if ( !$EventObj->canEdit() ) {
403+
throw new UnauthorizedException(__('Insufficient Privileges'));
404+
return;
405+
}
406+
382407
if ( $this->Event->delete() ) {
383408
//$this->loadModel('Frame');
384409
//$this->Event->Frame->delete();

web/api/app/Controller/FramesController.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,40 @@ public function beforeFilter() {
2626
}
2727
}
2828

29+
# Frames are addressed by their own Id, so the parent Event's per-monitor ACL
30+
# has to be resolved explicitly. Without this a user denied a monitor can
31+
# reach that monitor's frames by guessing frame Ids.
32+
private function eventForFrame($id) {
33+
$this->Frame->recursive = -1;
34+
$frame = $this->Frame->find('first', array(
35+
'conditions' => array('Frame.' . $this->Frame->primaryKey => $id)
36+
));
37+
if (!$frame) {
38+
throw new NotFoundException(__('Invalid frame'));
39+
}
40+
$this->loadModel('Event');
41+
$this->Event->recursive = -1;
42+
$event = $this->Event->find('first', array(
43+
'conditions' => array('Event.Id' => $frame['Frame']['EventId'])
44+
));
45+
if (!$event) {
46+
throw new NotFoundException(__('Invalid event'));
47+
}
48+
return new ZM\Event($event['Event']);
49+
}
50+
51+
# Frame mutation is an Event mutation, so require Events=Edit as well as the
52+
# per-monitor ACL. beforeFilter() only guarantees Events != None.
53+
private function requireFrameEdit($id) {
54+
global $user;
55+
if ($user and ($user->Events() != 'Edit')) {
56+
throw new UnauthorizedException(__('Insufficient Privileges'));
57+
}
58+
if (!$this->eventForFrame($id)->canEdit()) {
59+
throw new UnauthorizedException(__('Insufficient Privileges'));
60+
}
61+
}
62+
2963
/**
3064
* index method
3165
* @return void
@@ -67,6 +101,9 @@ public function view($id = null) {
67101
if (!$this->Frame->exists($id)) {
68102
throw new NotFoundException(__('Invalid frame'));
69103
}
104+
if (!$this->eventForFrame($id)->canView()) {
105+
throw new UnauthorizedException(__('Insufficient Privileges'));
106+
}
70107
$options = array('conditions' => array('Frame.' . $this->Frame->primaryKey => $id));
71108
$frame = $this->Frame->find('first', $options);
72109
$this->set(array(
@@ -102,6 +139,7 @@ public function edit($id = null) {
102139
if (!$this->Frame->exists($id)) {
103140
throw new NotFoundException(__('Invalid frame'));
104141
}
142+
$this->requireFrameEdit($id);
105143
if ($this->request->is(array('post', 'put'))) {
106144
if ($this->Frame->save($this->request->data)) {
107145
return $this->flash(__('The frame has been saved.'), array('action' => 'index'));
@@ -127,6 +165,7 @@ public function delete($id = null) {
127165
throw new NotFoundException(__('Invalid frame'));
128166
}
129167
$this->request->allowMethod('post', 'delete');
168+
$this->requireFrameEdit($id);
130169
if ($this->Frame->delete()) {
131170
return $this->flash(__('The frame has been deleted.'), array('action' => 'index'));
132171
} else {

web/api/app/Controller/ZonesController.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,19 @@ public function forMonitor($id = null) {
3131
if ( !$this->Monitor->exists($id) ) {
3232
throw new NotFoundException(__('Invalid monitor'));
3333
}
34+
35+
# Monitors=View is coarse. Enforce the per-monitor ACL so zones of a monitor
36+
# the user is denied are not listed by direct monitor Id.
37+
$this->Monitor->recursive = -1;
38+
$monitor = $this->Monitor->find('first', array(
39+
'conditions' => array('Monitor.Id' => $id)
40+
));
41+
$MonitorObj = new ZM\Monitor($monitor['Monitor']);
42+
if ( !$MonitorObj->canView() ) {
43+
throw new UnauthorizedException(__('Insufficient Privileges'));
44+
return;
45+
}
46+
3447
$this->Zone->recursive = -1;
3548
$zones = $this->Zone->find('all', array(
3649
'conditions' => array('MonitorId' => $id)

0 commit comments

Comments
 (0)