A live multipart (mode=jpeg) stream <img> whose baked auth hash expires
past AUTH_HASH_TTL is reconnected by the browser itself, reusing the same
src (same connkey, same dead hash). Every native reconnect returns 403, so
once the capture daemon drops the stream the client storms zms with auth
failures for hours. Observed in production: a single connkey retried 84
times over 2.5h, 880 failures on one monitor whose zmc was timing out,
while monitors with healthy zmc showed only baseline hash-rollover noise.
img_onerror only blanked the <img> src inside its async refresh callback,
which never ran once authRefreshAttempts reached the cap. On give-up the
stale src stayed live and the browser kept native-retrying it, which is the
storm. Blank src synchronously at the top of img_onerror so the browser's
retry loop stops immediately, and reconnect with a fresh connkey (the zms
process behind the old connkey has exited) after fetching a fresh hash.
Extract the src rewrite into a pure rebuildStreamSrc() helper in
auth-helpers.js with unit tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#4914 unconditionally drops Javascript errors and CSP violations sourced from
browser extensions. Some operators want to know when a plugin is touching the
ZoneMinder tab, so gate the suppression behind a config entry.
Add ZM_LOG_BROWSER_EXTENSIONS (boolean, default no) in the logging category.
It is exposed to JS by the existing non-private-config loop in skin.js.php as
the string '0'/'1' (the same convention as ZM_LOG_INJECT), so logger.js only
filters extension sources when it is '0'. Default behaviour is unchanged:
extension noise stays out of the log.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Streaming an event allocated and freed a ~2MB Image on every frame. Add a
reuse_image_ member and decode JPEGs into it via ReadJpeg, which reuses its
pixel buffer when dimensions match (every frame of a given event), on both
the JPEG and MPEG paths. The uncommon ffmpeg (mp4) path uses a unique_ptr so
it frees automatically without a manual delete or pointer-identity guard.
Replace the existence-check stat() calls, which filled an unused struct stat,
with std::filesystem::exists() using the error_code overload to keep stat's
non-throwing 'any failure means not present' behaviour.
This achieves the heap-allocation reduction of #4609 using modern C++ rather
than fixed char buffers and string_view. The per-frame path-string allocation
was already eliminated on master via the reuse_filepath_ member.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The build-rpm-packages workflows deploy with easingthemes/ssh-deploy using
rsync args -rltgoDzvO, which does not create missing parent directories on
the remote. When the zmrepo directory tree was deleted the deploy step
failed.
--mkpath is not a viable fix: it is parsed by the local rsync in the build
container, and Rocky 8 ships rsync 3.1.3 which predates the flag (3.2.3+).
Add a pre-deploy step that creates rpm/master/<family>/<releasever>/<arch>/
over ssh with mkdir -p, which has no rsync version dependency. Also add
utils/zmrepo_mkdirs.sh to recreate the full tree manually.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The command_processor thread was started in runStream() before
temp_image_buffer_count, the read/write indices and temp_image_buffer were
assigned, so a command arriving in that window observed a half-initialised
stream (the root cause behind the divide-by-zero guarded in the previous
commit). Keep the thread object declared up front for the join(), but defer
its launch until after the playback_buffer setup so processCommand can only
run against fully initialised state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MonitorStream::processCommand computed buffer_level by dividing by
temp_image_buffer_count (and via MOD_ADD modulo) while only guarding on
playback_buffer. processCommand runs on the command_processor thread,
started in runStream before temp_image_buffer_count is assigned from
playback_buffer. In that window temp_image_buffer_count is still 0 while
playback_buffer is already > 0, so a command arriving then divided by
zero and raised SIGFPE, crashing nph-zms.
Extract the percentage math into MonitorStreamBufferLevel which returns 0
when the buffer count is not positive, and add Catch2 coverage for the
zero-count and wrap-around cases.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
When an event closes the recording file is renamed from incomplete.* to
<Id>-video.* and the DB row is updated. A stale Event model still reports
DefaultVideo=incomplete.mp4, producing spurious 'File does not exist'
warnings. When the incomplete file is missing, clear the object cache and
re-read the event from the database to check the finished file before
warning.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- zm_libvnc_camera.cpp resize(): return FALSE and log if av_malloc fails,
rather than returning TRUE with a null frameBuffer (libVNC would then
write into it and crash). Matters more now that size_t sizing can
request large allocations for server-advertised dimensions.
- zm_libvnc_camera.cpp: compute the buffer sizes in the scale Debug() in
size_t with %zu, so the logged sizes can't overflow int and match the
values passed to SWScale::Convert.
- zm_libvlc_camera: widen LibvlcPrivateData::bufferSize to size_t and
compute it in size_t in PrimeCapture, so the allocation itself can't
overflow. Image::Assign now passes the same stored bufferSize used for
the allocation, so the read size can't exceed the buffer. Widen the
compare loop index to size_t to match.
Verified: both translation units compile with -Werror -fsyntax-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address PR #4931 review feedback:
- monitor.js ControlEdit_onClick: parse ControlId as an integer and only
navigate when it is a positive integer, instead of URL-encoding the raw
select value.
- skin.js .zmlink handler: drop the unused binding from the catch clause.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CodeQL's open alerts are dominated by findings inside bundled third-party
libraries (jQuery UI, Bootstrap 4, bootstrap-table, the jQuery UI
timepicker addon, hls.js). These flag coding patterns internal to those
libraries -- js/unsafe-jquery-plugin, js/insecure-randomness, etc. -- that
are not ZoneMinder bugs and cannot be fixed without forking the
dependencies. They drown out findings in ZoneMinder-authored code.
Add the vendored library directories/files to paths-ignore in the CodeQL
config. ZoneMinder-authored files in these trees (skin.js,
MonitorStream.js, views/js/*.js, ...) are not listed and remain analysed.
moment.js is intentionally left out: it is scheduled for removal once its
remaining call sites migrate to luxon, so its alert will be resolved by
deletion rather than suppression.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zmwatch only gated the camera reboot attempt on CanReboot(), so it called
$control->open() even when the camera was unreachable - the common reason a
monitor has no image since startup - and blocked until the connection timed
out, logging an error each pass.
Add a ping check before open(). Move the host resolution into Control so callers
don't have to dig the ip out of the Path: add Control::host(), which returns the
cached host or derives it from the monitor's ControlAddress/Path via the shared
guess_credentials() (parsing only, no network i/o), and have ping() fall back to
it. ping() still accepts an explicit ip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
generateAuthHash() cached every hash under the IP-keyed slot
'AuthHash'.$remoteAddr, but the value was IP-bound only when $useRemoteAddr was
set. getZmuCommand() calls generateAuthHash(false, true), which wrote an
IP-less hash into the IP-bound slot and reset AuthHashGeneratedAt. Because the
status poll runs getZmuCommand (web/ajax/status.php) right before emitting the
auth hash, the poll then served the IP-less hash to the browser; the next
IP-bound request was rejected by the validator, redirecting the user to login
roughly every poll. This happens with a completely stable client IP, so it is
distinct from the IP-rotation case.
Key the cache slot by the address actually baked into the value: only use the
session address when this caller asked for it (and AUTH_HASH_IPS is on). IP-less
and IP-bound hashes now occupy separate slots and can no longer clobber each
other.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
zmwatch only gated the camera reboot attempt on CanReboot(), so it called
$control->open() even when the camera was unreachable - the common reason a
monitor has no image since startup - and blocked until the connection timed
out, logging an error each pass.
Add a ping check before open(). Move the host resolution into Control so callers
don't have to dig the ip out of the Path: add Control::host(), which returns the
cached host or derives it from the monitor's ControlAddress/Path via the shared
guess_credentials() (parsing only, no network i/o), and have ping() fall back to
it. ping() still accepts an explicit ip.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Primary change: remove web/js/hls-1.6.13/hls-demo.js (+ its source map),
the upstream demo bundle. It is referenced nowhere in the web interface
and is the source of four CodeQL alerts (js/xss-through-dom,
js/incomplete-sanitization, js/redos: #262-#265).
Optional cleanup (can be dropped from this PR if maintainers prefer to
keep the full dist): the directory also shipped alternate builds that the
web interface never loads. The interface only loads hls.min.js
(watch.php, cycle.php, montage.php). Also removed:
- hls.js, hls.light.*, hls.mjs, hls.light.mjs (+ maps): alternate
full/light/ESM builds; we use the full minified UMD build
- hls.worker.js (+ map): standalone transmuxer worker. hls.min.js inlines
the worker via a Blob URL and only loads an external worker when
config.workerPath is set; ZM never sets it, so it is never fetched
- hls.d.mts, hls.d.ts, hls.js.d.ts: TypeScript declarations, dev-only
Keeps hls.min.js and its source map. The demo removal alone clears
#262-#265; trimming the unused builds additionally clears the
js/insecure-randomness alerts in them (#258-#261).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves three CodeQL alerts in ZoneMinder-authored JavaScript:
- skin.js .zmlink click handler (js/xss-through-dom, #135): navigated to
the element's href/data-url via location.assign() without checking the
scheme, so a javascript:/data: URL in the attribute would run on click.
Skip navigation for javascript:/data:/vbscript: URLs.
- skin.js strip_html() (js/incomplete-multi-character-sanitization #266,
js/polynomial-redos #267): stripped tags with /<[^>]+>/g, which is an
incomplete sanitizer and can backtrack. Parse the string with DOMParser
and return textContent instead -- correct and linear. Callers in
devices/reports/snapshots/controlcaps/events.js use it to extract a
plain Id from a rendered cell, behaviour preserved.
- monitor.js ControlEdit_onClick (js/xss-through-dom, #246): encode the
select value with encodeURIComponent() before building the controlcap
URL.
Verified: eslint clean on both files, node --check passes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The frame buffer size calculations in the libVNC and libVLC camera
backends multiplied 16/32-bit operands and only widened the result to
size_t afterwards, so the multiplication itself could overflow before
the conversion. Flagged by CodeQL cpp/integer-multiplication-cast-to-long
(alerts 282, 145, 85). The VNC framebuffer dimensions are advertised by
the remote server (uint16_t), making the inputs externally influenced.
Cast the first operand to size_t so each product is computed in size_t:
- zm_libvnc_camera.cpp: SwScale::Convert src size (framebufferWidth *
framebufferHeight * 4) and dest size (width * height * colours)
- zm_libvnc_camera.cpp: resize() bufferSize, changed from int to size_t
(not converted to a larger type, so CodeQL did not flag it, but the
same overflow applied); Debug format updated to %zu
- zm_libvlc_camera.cpp: Image::Assign size (width * height * mBpp)
Verified: both translation units compile with -Werror -fsyntax-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The hand-rolled regex in open() mis-parsed a stream Path that carried no
credentials, e.g. rtsp://10.0.0.4:554/cam: the greedy [^:@]+ consumed the host
as the username, 554 as the password, and ADDRESS backtracked to the single
digit 4. The control daemon then dialled http://...@4, producing
"Can't connect to 4:80 (Connection timed out)".
Replace the regex with the shared URI-based Control.pm guess_credentials(),
which handles ControlAddress and Path, converts rtsp to http, and falls back to
the Monitor User/Pass. Create the LWP::UserAgent before parsing since
guess_credentials() sets credentials on it. The Grandstream login wire protocol
(challenge/authcode and old-style fallback) is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The tablename field of a filter term was copied verbatim into SQL by
sql_attr(), while attr, op, val and collate were all sanitized. An
authenticated user with Events View permission could inject SQL via the
filter[Query][terms][N][tablename] request parameter, enabling blind
read access to the whole database (password hashes, camera credentials).
Restrict tablename to the table aliases actually used by the filter
queries (E, M, S, F, T, ET, Snapshots); reject anything else, log it,
and fall back to 'E'.
Refs GHSA-q2w3-h644-f8xq
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Avoid sleeping up to MAX_SLEEP (500ms) at the end of runStream when
termination has been requested, so zms shuts down more promptly.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
zm_terminate is written from signal handlers and read from daemon loops.
A plain bool is undefined to access from a signal handler and may be hoisted
out of a loop by the optimizer. std::atomic<bool> is lock-free on supported
platforms and matches the already-atomic per-object terminate_ flags. Update
the three Debug() vararg sites to call .load() since atomics can't pass
through ...
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On systems with many cameras the Grandstream control log entries gave no
indication of which monitor they referred to. Prefix every Debug/Warning/Error
message with the monitor Id and Name. Also fix two message typos
(challengstring, UNable).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CakePHP applies the datasource 'encoding' as SET NAMES, and it was 'utf8',
MySQL's 3-byte utf8mb3 alias. Like the C++ daemon connection, this mangles
4-byte UTF-8 characters in utf8mb4 columns such as Monitors.Name to '?' on
read and truncates them on write, so the API returned and stored corrupted
names. Set it to utf8mb4 to match the schema.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The connection charset was "utf8", MySQL's alias for 3-byte utf8mb3, while
Monitors.Name and the DB default are utf8mb4. On a utf8mb3 connection any
4-byte UTF-8 character is read back as '?' and, when embedded in a log message
written to the Logs table, raises ERROR 1366 and truncates the value. Names
with such characters lost their multibyte portion in logs, keeping only the
ASCII tail.
Connect with utf8mb4, falling back to utf8 with a warning if the server
rejects it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
