finalize() located the mfra by reading the trailing mfro with fopen and
fseeko and decoding it by hand, the same job zm_mp4::media_end() does for
the sidx scan. Use media_end() for both, so the last HLS fragment and the
index agree on where the media ends.
media_end() is also stricter: it requires an mfra box of the stated size
at the offset the mfro points to, where the old code accepted any size up
to the file length. A trailer that does not check out now leaves the final
fragment running to EOF, as a missing trailer already did.
A new test covers media_end() against the fixture's real mfra and three
damaged trailers: an mfro size off by four, one larger than the file, and
no mfro at all.
refs #5144
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
build_sidx_region() wrote starts_with_SAP=1, SAP type 1 into every
reference. That holds for frag_keyframe recordings, where every fragment
opens on a keyframe, but a monitor whose encoder options add frag_duration
or frag_size gets fragments cut mid-GOP, and the index then claimed a
decodable start where there was none.
scan_fragments() now reads the sync flag of each fragment's first video
sample: trun first_sample_flags, else the first entry's sample_flags, else
the tfhd default_sample_flags, else the trex default (read_video_track()
now keeps it). A reference with no SAP gets 0 for starts_with_SAP, type and
delta; a merged reference takes the flag of its first fragment. parse_traf()
also skips tfhd default_sample_size, which it never needed before.
Tests: remuxing the fixture with frag_keyframe and a 250 ms frag_duration
gives about a dozen references, of which exactly the three that begin at a
keyframe are SAPs; the frag_keyframe fixture scans as all SAPs, and the
byte-for-byte comparison with the reference tool is unchanged; merging takes
the first fragment's flag. The remux sequence the muxer test used is now a
helper shared by both.
refs #5144
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every fragmented event reserved 64 KiB for its leading sidx, whatever its
length. On a short motion clip of 1-2 MB that is 3-6% of the file.
zm_mp4::reserve_size() sizes the region for twice the fragments expected
in one section: section length divided by the GOP, which is the encoder's
gop_size when encoding and the packet queue's longest keyframe interval
otherwise, over the capture fps. It rounds up to whole 4 KiB blocks and
stays between 4 KiB (337 references) and the previous 64 KiB, which is
still taken when the section length, GOP or fps is unknown. The default
600 s section at a 1 s GOP now reserves 16 KiB.
A low guess does not lose the index: an event with more fragments than
the region holds has neighbouring fragments merged into one reference,
as before, which only coarsens seeking. VideoStore keeps the size it
reserved so finalize() fills that exact region, and Monitor gains a
GetSectionLength() accessor.
refs #5144
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cc382b040 made the non-Linux fallback of sync_range() call fdatasync(),
which macOS's unistd.h does not declare, breaking the macOS build:
zm_mp4_sidx.cpp:120:10: error: use of undeclared identifier 'fdatasync'
Use fsync(), as the code did before that commit and which built on macOS
in #5145's CI. Linux still syncs only the region with sync_file_range().
refs #5144
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
write_leading_sidx() called fsync() twice when an event closed. The first
wrote back every dirty page of the event file -- the whole recording is
usually still in the page cache at close, often hundreds of MB -- only to
order the 64 KiB region body before its 8-byte header. On a 400 MB file
on NVMe the two calls took 0.24 s; on a disk writing 100 MB/s it is about
4 s per event, and closeEvent() joins the previous close thread, so
back-to-back events could stall analysis.
On Linux, sync_file_range() now writes back just the region body before
the header goes in, which takes under a millisecond for the same file. It
waits for the write-back but does not flush the drive cache. Other systems,
and filesystems that reject the call, fall back to fdatasync().
The sync after the header is dropped. If that write is lost, the region is
still the free box it was, so durability of the header is not needed for a
valid file; the old code also logged "cannot flush" and reported failure
for an index that had been written.
refs #5144
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
VideoStore::open() reserves a 64 KiB free box between moov and the first
fragment, but only when the MOV muxer wrote the moov in the header and
moof+mdat fragments follow it. finalize() scans the fragments and fills the
region with free padding followed by a sidx that ends at the first moof, so
FFmpeg-based players (Chromium, Android WebView, Electron) treat the index as
complete and start playback without visiting every fragment. On any parse or
write failure the region stays a free box.
fixes#5144
51e8a4571 dropped libcurl4-gnutls-dev from the runtime Depends, relying on
${shlibs:Depends} for the library. zoneminder-containers/zoneminder-base
builds its runtime dependency package from the control file text with
equivs, where ${shlibs:Depends} is never expanded, so the image lost
libcurl-gnutls.so.4 and zmc/zma exited with status 127.
Add libcurl3t64-gnutls | libcurl3-gnutls, the runtime library the build
links against (libcurl4-gnutls-dev is the first Build-Depends choice).
It is a library package, so it does not conflict with
libcurl4-openssl-dev the way the -dev package did.
Refs zoneminder-containers/zoneminder-base#89
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
get_event_start_packet_it() logged "Hit end of packetqueue before
satisfying pre_event_count" at Debug when packet->image_index was below
pre_event_count, assuming that meant zmc had just started. image_index
comes from shared_data->image_count, which is not reset when a capture
reconnect runs Monitor::Pause() and clears the packetqueue. An event
opened on the first frames after a reconnect therefore logged a Warning
for a short pre-event buffer that cannot be avoided.
Record the queue_index of the first packet queued since the last clear()
and log at Debug when the walk back stops on that packet, i.e. nothing
has been trimmed since startup or the reconnect. A Warning now means
packets really were removed below the pre-event count.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Snapshots index/view/associations returned each snapshot's events without
a monitor check, and Snapshots and Tags add/edit attached any event ids,
so a user could add a denied monitor's event to a snapshot and then read
it back. Attaching now needs view on each event, listed events are
filtered to viewable monitors, and ids are pinned. Tags' existing
filters also match nothing, rather than everything, for a user denied
every monitor.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Group membership feeds per-monitor access through Groups_Permissions, but
the classic group actions and the Groups API checked only the global
Groups permission. A Groups editor could add a monitor they are denied to
a group they have access through, or remove it from, or delete, the group
that denies it.
Add Group::canEditMembership() and require it, in both the classic UI and
the API, for every monitor added to or removed from a group, for all of a
group's monitors when it is re-parented, and for all of them when it is
deleted. The API also pins the record id and omits monitors the user may
not view from the groups it lists.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LogsController::delete() never declared global $user, so its System=Edit
check always passed and anyone with System view could delete log entries.
Logs add, which ZM_LOG_INJECT opens to non-admins, could overwrite an
existing entry by Id; pin it.
ZonePresetsController had no permission checks. Reading presets stays
open to signed-in users; changing them now needs System=Edit.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The API alarm action and ajax/alarm.php checked no per-monitor
permission and relied on zmu, which only requires that the user can see
the monitor. A user with view on a monitor could force, cancel or disable
its alarms. Changing alarm state now needs Monitor::canEdit(), and the
API status query needs canView().
Monitors API edit and add also pin the record id, so an Id in the body
cannot redirect the save to another monitor.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- 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>
AppController mapped index/add/edit/view/keyvalue/category to Crud
actions, and CrudControllerTrait answers any action a controller does not
define with them. Those generic handlers apply none of the controller's
permission or per-monitor checks: zones/view/<id> and zones/<id> returned
any zone, including those of monitors the user is denied, and Controls
add/edit and Configs add were reachable the same way. Map no Crud actions,
so an undefined action is a 404, and give ZonesController a view() that
checks the zone's monitor.
ZonesController::index() passed its monitor filter as a find() option key
rather than a condition, so it was ignored and every zone was listed. Use
a real condition.
Add AppController helpers the following fixes share: a viewable-monitor
find() condition that matches nothing when the user may view no monitor
(callers treated an empty list as unrestricted), reading a field or
associated ids from request data, and requiring view or edit on a monitor
or view on events.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZM_AUTH_HASH_SECRET signs the JWT access and refresh tokens, and its
default is a fixed string in the public source. Nothing generated a
per-install value and tokens signed with the default verified normally,
so on an install with auth on and the secret untouched anyone could sign
an admin token and be accepted by the web UI, the API and zms.
- ZoneMinder::Config::saveConfigToDB() now replaces an empty or default
secret with 32 random bytes from /dev/urandom, hex encoded. Package
installs and upgrades run zmupdate.pl -f, which saves the config, so
existing installs get a secret on upgrade. A secret the admin set is
left alone.
- validateToken() in PHP and zmLoadTokenUser() in C++ refuse to verify
tokens while the secret is empty or the default, and the API refuses to
issue them, as it already did for an empty secret.
- zmLoadTokenUser() no longer writes the signing key to the debug log.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zms checked access against the monitor id in the query string and then
streamed the event id from the same query, loading the event's real
monitor later without checking it. A viewer allowed monitor 1 and denied
monitor 2 could fetch a monitor-2 event by sending monitor=1, or by
leaving monitor out, which checked only the global Events permission.
For event requests, look up the event's MonitorId and require Events
view plus access to that monitor. When the stream is chosen by monitor
and time instead of by event id, the named monitor is the one streamed
and is the one checked. Moving to another event during playback stays
within the same monitor, so checking the first one is enough.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ReadJpeg() stored the JPEG header's width and height in the Image before
calling WriteBuffer(). WriteBuffer() reallocates only when the requested
size differs from the current one, so it saw no change and kept the
existing buffer and linesize, and the scanline loop then decoded every
row of a larger file past its end. A File monitor re-reads its source
into a monitor-sized image on every capture, so whoever can write that
file could overflow zmc's heap. Stored event JPEGs read by zms take the
same path.
Leave width and height to WriteBuffer(), as DecodeJpeg() already does.
FileCamera::Capture() also now refuses a file whose dimensions do not
match the monitor, since everything downstream is sized for the monitor.
Add a Catch2 case that reads a 256x192 JPEG into a 64x48 image. It
segfaulted before this change and passes after.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The filter view normalised filter[Id] only when no top-level Id was
given, and filter[...] is then applied to the filter object. With Id=1
in the URL, a crafted filter[Id] reached filter.js.php unchanged and
was echoed into a single-quoted string in the page's nonce-bearing
script, giving reflected script execution from a link.
Always normalise filter[Id], let the top-level Id win when both are
given, and escape the id where the view writes it into script and HTML.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Monitor.DefaultPlayer is a free varchar that the API saves unvalidated.
The montage, watch, zone, zones and cycle scripts echoed it inside a
single-quoted JavaScript string, so a monitor editor could store
x',p:alert(1),z:' and run script as anyone viewing that monitor, inside
the page's nonce-bearing script.
Pass every string-valued monitor field these templates emit through
validJsStr(): DefaultPlayer, StreamChannel, Janus_Pin (which comes from
the Janus server), WhatDisplay, Type, Capturing, Refresh and the initial
scale, in montage, watch, zone, zones, cycle, montagereview and event.
In montage, re-encode the stored layout Positions with JSON_HEX_TAG and
friends instead of echoing the stored JSON, since a string in it could
otherwise close the script, and escape autoLayoutName. Console's
data-stream-channel attribute gets validHtmlStr().
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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>
ZonesController add/edit/delete checked only the global Monitors
permission, so a Monitors=Edit user denied a monitor could still add,
rename, reshape or delete that monitor's zones by id.
Each now requires Monitor::canEdit() on the zone's current monitor, and on
the MonitorId in the request data for add and for an edit that moves the
zone. add() rejects a request without a MonitorId.
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>
The classic event actions checked only the global Events permission and
then acted on whatever event ids the request supplied, so a user denied a
monitor could still change that monitor's events:
- deleteEvent() deleted when the user had Events=Edit. It now requires
Event::canEdit(), which also requires access to the event's monitor.
This covers the events form, the event form and monitor deletion.
- The events form archive/unarchive updated Events by id. Each event now
needs canEdit().
- ajax events archiveRequest() updated by id under the page-wide Events
view check. Archive now needs canView() on the event and unarchive
canEdit(), keeping the intent that viewers may archive.
- actions/event.php returned early whenever an eid was supplied, so none
of its actions ran. Fix that inverted test, and require canEdit() on the
event for rename, detail edits, archive, unarchive and delete, rather
than the global permission.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZM_Object::set() called any method whose name matched a key in its data,
and changes() called it as a getter. That data is usually a request array
(filter[...], newMonitor[...], user[...]), so a request could invoke
save(), delete(), execute() and the like. filterdebug with fid=0 did
exactly that before its authorization check: filter[save][...] stored an
AutoExecute filter with a chosen command and filter[execute] ran
zmfilter.pl on it, giving command execution to any logged-in user. The
filter and events views pass filter[...] to set() the same way.
set() and changes() now dispatch a key to a method only when the key is
a field in $defaults or is listed in the class's new static $setters, the
accessors outside $defaults that take a value (Filter's query accessors,
Monitor::Model/Manufacturer/Groups, User::Role, and so on). Other method
names are refused with a warning.
filterdebug also requires Events view before it builds a filter from the
request.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
712994b19 stopped writing sessions for requests that arrive without a
ZMSESSID cookie unless zm_session_persist() marks them as being issued.
Web login goes through zm_session_regenerate_id_login(), which marks the
session, but the legacy API login with stateful=1 authenticates via
validateUser() and only calls zm_session_start(). A client logging in that
way without a cookie was handed a ZMSESSID cookie for a session that was
never stored, so its following cookie-only requests were unauthenticated.
Mark the session persistent once the stateful login has a user.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getNearEvents compared only the sort column with >= / <= and excluded the
current event, using Id solely as a secondary ORDER BY. When several events
share a sort value - most often StartDateTime, which has one-second
resolution, with multiple monitors recording in step - the later event of
a tie chose the earlier one as its Next. Event playback, gapless or not,
then looped between the two and the Next button could not escape.
Compare the full (sort value, Id) tuple so ordering is strict in both
directions. Read the current event's sort value through the same
Events/Monitors join as the searches, which also makes Prev/Next work
when sorting by MonitorName (the value was previously looked up in the
Events row and was always NULL).
There is no need to duplicate #progressBar in the dark theme.
Furthermore, after the level graph was added, the height of #progressBar was adjusted in the base theme but not in the dark theme, resulting in incorrect rendering.
These three blocks stacked under the event video and each took a full
line. Wrap them in a new #eventControls flex row: monitor name left,
transport buttons centred, replay status right, wrapping onto separate
lines when the viewport is too narrow.
The span#rate was never closed, so #progress, #currentTime, #zoom and
#fps were nested inside it. Close it, otherwise the whole status line
lays out as a single flex item.
The three children are flex containers themselves so their contents
centre vertically while the boxes stay stretched to the row height.
scaleToFit() measures the bottom edge of #replayStatus to decide how
tall the video can be, and stretching keeps that edge at the bottom of
the row, so auto scaling is unchanged.
The new rules are scoped under #eventControls, which outranks the
duplicated single-id rules in the classic and dark event.css.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tag input and its prev/next buttons spanned the full width of the
event view above the video row. Move the tags-container into the
eventStats column and narrow that column from col-sm-4 to col-sm-3 so
the video gets the width back.
- tags-container drops its 1rem side margins, which were sized for a
full-width bar, and gains bottom spacing to separate it from the
stats table.
- tag-dropdown gets position: relative so the suggestion list anchors
under the input. It had no positioned ancestor before, so in the
narrower column the absolutely positioned list would have landed in
the wrong place. It also gets a min-width so the input stays usable
once tag chips wrap.
- tag-dropdown-content sizes to its items rather than to the narrow
input.
- tag-input can shrink inside the flex row instead of forcing overflow.
event.js toggles the video wrapper between the stats-visible width and
col-sm-12 when the stats column is hidden, so update those six places
to col-sm-9 as well.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
icons is the toolbar icon map that thirteen view scripts, and logpanel.js,
pass to bootstrapTable(). It was defined in skin.js, which xhtmlFooter()
loads last of all, after every view's own script. A client that loads the
whole page is fine, because skin.js still runs before DOMContentLoaded,
but one that stops short -- a crawler that caps resource loading, an
aborted load -- runs the view's ready handler against an undefined icons
and the table never initialises:
Uncaught ReferenceError: icons is not defined
at HTMLDocument.initPage (.../views_js_console-....js:458:12)
Move it to web/js/table-helpers.js, which already exists to hold what the
bootstrap-table views share and is loaded well before them, and export it
alongside the other helpers for the node tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CSRF token check on the image proxy only runs when ZM_ENABLE_CSRF_MAGIC
is on. Browsers that send Sec-Fetch-Site report when another site started a
request, so refuse proxy requests whose value is anything but same-origin or
none, whatever the CSRF setting. Browsers that do not send the header are
unaffected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Event::GenerateVideo() built the ffmpeg command as one string and ran it
through escapeshellcmd(), which leaves spaces alone. Any value in the string
could therefore add ffmpeg arguments. The event Name is already reduced to a
safe filename, but DefaultVideo and the transforms were not.
The command is now a list with every argument passed through escapeshellarg(),
the same approach as the Perl GenerateVideo. The configured input and output
option strings are split on whitespace, as the Perl side does. ffmpeg output
goes to ffmpeg.log through proc_open descriptors: escapeshellcmd() was escaping
the old '> ffmpeg.log 2>&1' redirect, so it was passed to ffmpeg as arguments.
An empty transforms string no longer adds a bare -vf.
Checked with a stub ffmpeg that records its argv: a Name and a DefaultVideo
carrying shell and option payloads arrive as single literal arguments and
nothing runs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fileSize() joined the stored DefaultVideo onto the event directory as is,
so an older row, or one written before the model validation was added,
holding ../ could stat a file outside it. fileExists() above already
applies basename(); do the same here.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EventStream::loadEventData and zma both joined the DefaultVideo column
onto the event path and opened the result, so a DefaultVideo containing
../ made zms stream, or zma read and hard link, files outside the event
directory. Reduce it to std::filesystem::path::filename() when loading.
zma also wrote the value back into a new event's row with plain
string formatting; escape it with zmDbEscapeString.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 18f48f8bc95f53d0b21110f2f5e5a2c9d22c9a9b)
GenerateVideo used DefaultVideo as the ffmpeg input relative to the event
directory and built the output file from Name with only whitespace
replaced, so an Events=Edit user could make zmvideo read or write any
path the account can reach. zmfilter's generateImage passed
event_path/DefaultVideo to ffmpeg -i, and the %EV% email tag attached
event_path/DefaultVideo, which could mail out any readable file.
Strip everything up to the last path separator from DefaultVideo in all
three places, and reduce Name to [-A-Za-z0-9_.] with no leading dot
before using it as the video filename.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 5904023f13448e557dc205edb9440972e4a6482d)
DefaultVideo can be set by any user with Events=Edit through the API
(EventsController add/edit), and readers join it onto the event path:
view_video.php streams it, image.php extracts frames from it,
findVideoEventFile, Event::getStreamSrc/FileSize and the API fileExists
check all use Path().'/'.DefaultVideo. A value such as ../../x pointed
those reads at arbitrary files readable by the web account.
Reject DefaultVideo values on save in the API model unless they are a
bare filename (no / or \, no NUL, not . or ..). Make the PHP Event
DefaultVideo() accessor return basename() so every web reader only ever
looks inside the event directory, even for rows written before this
check, and apply basename() to the raw array read in the API model's
fileExists.
Event::GenerateVideo built its output filename from Name with only
whitespace replaced; Name is editable through the event rename and
eventdetail actions. Replace anything outside [-A-Za-z0-9_.] and a
leading dot so the output stays inside the event directory.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit f3f679b644db28b4b1bff79d5dd98eca24395fc4)
