FilterComponent::buildFilter() checked the operator against an allow list
but never the field, so an unrecognised name went straight into the SQL:
GET /api/events/index/MonitorName =:Monitor-1.json
PDOException SQLSTATE[42S22]: Column not found: 1054
Unknown column 'MonitorName' in 'where clause' -> HTTP 500
A caller filtering on a monitor-scoped attribute (MonitorName, Monitor,
Rack, Experiment, Server, Storage) or simply mistyping a column got an
opaque 500 with a stack trace and nothing naming the offending field.
Take an optional list of permitted fields and reject anything else with
BadRequestException. EventsController::index() passes the Events columns
from the model schema plus the two names it resolves itself: DateTime, the
pseudo-attribute it turns into an overlap test, and GroupId, served by the
Groups_Monitors join. A field written Model.Field is checked on the column.
The two other callers of buildFilter pass no list and are unchanged.
The invalid-argument and invalid-operator paths threw a plain Exception,
which was also a 500 for what is bad input; they throw BadRequestException
now too.
Verified against a live install:
MonitorName / Rack / NotAColumn 400, "Unknown filter field: <name>"
DateTime >=/<= over a window 200, 7 rows, overlap intact
Cause, Event.Cause, GroupId 200
?limit=2&sort=...&direction=desc 200, 2 rows, pagination unaffected
Plus an assert script over the validation itself, including that a caller
passing no list keeps the previous pass-through behaviour.
Filters sent as a query string rather than named parameters go through a
different branch, on raw $_REQUEST, and are deliberately left unvalidated:
that array also carries auth and cache-busting parameters, so rejecting
unknown names there would break existing callers. ?MonitorName=x is still
a 500 and wants its own change.
(cherry picked from commit 4d82e1d385)
- index/view, Frames index and EventData index/view treated an empty
viewable-monitor list as no restriction, so a user denied every monitor
saw all of them. They now match nothing in that case.
- search and consoleEvents had no monitor filter at all and returned
events and per-monitor counts for denied monitors.
- add saved any MonitorId; it now needs view on that monitor, as editing
an event does, and cannot update an existing event named in the body.
- edit checked the event in the URL but saved the body, which could name
another event's Id or move the event to a denied monitor. Pin the id and
check a new MonitorId.
- createThumbnail and getMaxScoreAlarmFrameId were public, so routable,
and checked nothing. They are internal helpers; make them private.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 1ba42e8710)
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>
(cherry picked from commit 516bab0247)
The REST API archive action toggled an event's Archived
(retention-protection) flag with no authorization beyond the
controller's coarse "Events permission is not None" gate. Any
authenticated read-only user, including one restricted to a subset of
monitors, could flip the retention state of any event by enumerating
event ids, and the action was reachable over GET (CSRF-able).
Gate the write by direction: archiving (protects from purge) requires
view access via Event::canView(); un-archiving (re-exposes to purge)
requires edit access via Event::canEdit(). Both enforce the per-monitor
object-level ACL. Restrict the action to POST/PUT to block CSRF.
Addresses GHSA-5v9h-ww7p-hxgv.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FilterTerm.php:
- Replace eval() with safe compare() method for SystemLoad, DiskPercent,
and DiskBlocks filter conditions (RCE via crafted op/val)
- Validate operator against allowlist in constructor
- Sanitize collate field to alphanumeric/underscore only (SQLi)
onvifprobe.php:
- Use escapeshellarg() on interface, device_ep, soapversion, username,
and password arguments passed to execONVIF() (command injection)
Event.php:
- Use escapeshellarg() on all arguments to zmvideo.pl instead of
escapeshellcmd() on the whole command (command injection via format)
- Anchor scale regex with ^ and $ to prevent partial matches
image.php:
- Restrict proxy URL scheme to http/https only (SSRF via file:// etc)
filterdebug.php:
- Use already-sanitized $fid instead of raw $_REQUEST['fid'] (XSS)
MonitorsController.php:
- Use escapeshellarg() on token, username, password, and monitor id
in zmu shell command instead of escapeshellcmd() on whole command
HostController.php:
- Use escapeshellarg() on path in du command (command injection via mid)
- Remove space from daemon name allowlist (argument injection)
EventsController.php:
- Remove single quotes from interval expression regex (SQLi)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Populate a global from the session on every request. Use the object instead of using allowedMonitors in session.
* fix when gets loaded.
* use for auth, and add Monitor Edit checks to Zone add/delete/edit
* add back the ZM_OPT_USE_AUTH test for being logged in in AppController
* Update permissions code to use
* change quotes
* Update permission code to use
* Use instal of session for systemPermission
* deprecate montiorPermision in session
* use instead of session streamPermission
* move login code back into AppController. Has to be done for every request
* deprecate eventPermission, controlPermission and systemPermission in session.
* handle auth params in query string as well as post
* exit on HUP to free up memory.
* add missing global user
* system should be System
Isaac Connor