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.
DateTime is a pseudo-attribute meaning "the event was running then", so a
window over it should select events that overlap the window. The code applied
the term's own operator to both StartDateTime and EndDateTime, which turns the
upper bound into StartDateTime <= max AND EndDateTime <= max -- a containment
test. Any event spanning the end of the window was dropped, which under
continuous recording is most of them, and with events longer than the window,
all of them. Montage review showed an empty timeline as a result.
The lower bound now tests the event's end and the upper bound its start.
The EndDateTime IS NULL allowance is also bounded. It was there so an event
still being written stays visible, but as written it made every crash-orphaned
event ever recorded match every window: montage review was returning events
from two weeks earlier and nothing from the requested hour. An event with no
EndDateTime still has Length, flushed every few seconds by zmc, so
StartDateTime + Length is its effective end; only an event with neither falls
back to NOW(). This is the same expression the Event model already uses for
its EndTimeSecs virtual field.
Verified against a live instance: for a 09:43-10:43 window that had one
overlapping event per monitor, the API returned 8 events for monitor 1, all
crash orphans from 2026-07-26 and none from the window. It now returns the
overlapping event, and montage review draws it and renders its frames.
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>
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>
The view() action sets recursive=1 on the Event model, which the
subsequent find('neighbors') calls inherited. That made each of the four
neighbor lookups (prev/next, prevOfMonitor/nextOfMonitor) SELECT every
column from Events plus LEFT JOIN Monitor and Storage, then fire a
separate Frames hasMany query per neighbor row. Only Event.Id is used
downstream.
Pass fields=Event.Id and recursive=-1 on each neighbor call so the
generated SQL is just:
SELECT Event.Id FROM Events AS Event WHERE Event.Id < ?
ORDER BY Event.Id DESC LIMIT 1
The per-monitor variant uses Events_MonitorId_idx which already covers
(MonitorId, Id) via InnoDB's implicit PK suffix, so no schema change is
needed.
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
singhharsh1708