web/skins/classic/includes/config.php defines all 18 ZM_WEB_* constants inside
a switch on $_COOKIE['zmBandwidth'] with cases for high, medium and low and no
default. On any other value none of them are defined, and skin.js.php - emitted
in the footer of every page - reads ZM_WEB_VIEWING_TIMEOUT, ZM_WEB_AJAX_TIMEOUT
and ZM_WEB_REFRESH_NAVBAR. On PHP 8 an undefined constant is a fatal Error, so
every page including login stops rendering until the cookie is cleared, which
cannot be done from inside the interface.
skin.php only tested the value for empty, and nothing else validated it:
- the cookie is set client side by skin.js, so any value survives
- action=bandwidth put $_REQUEST['newBandwidth'] through validStr, which is
only strip_tags, and persisted it
- ZM_BANDWIDTH_DEFAULT is a free-form string in ConfigData. The Options UI
renders it as a select, but loadConfig lets a conf.d file override the
database, so a typo there locks out everyone with no cookie yet
skin.php now validates both the cookie and ZM_BANDWIDTH_DEFAULT before falling
back to low, and the action rejects a value it does not recognise rather than
storing it.
tests/php/test_bandwidth_clamp.php checks the whitelist against the switch it
guards - the two must name the same profiles, since a value in one and not the
other reopens this - and that no arm of the switch defines a constant the
others do not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y6FieTwEXuLhhR4e2yiax
logState() counted every row with Level < INFO and folded anything at or
below PANIC into the FATAL bucket. AUDIT is -5, so audit entries were
counted as fatals and pushed the state to alert/alarm.
Bound the count query at PANIC so AUDIT and NOLOG are left out.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GAFKf86P78WqniPEP2b45J
Two cases produced a Warning for a request that needs no CORS headers at
all, so the log filled with noise that pointed at nothing wrong.
An empty Origin header satisfies isset(), so CORSHeaders() walked the
servers list, matched nothing, and logged " is not found in servers list."
with no value to print. Treat an empty Origin the same as no Origin.
Browsers also send Origin on same-origin POST and fetch. Such a request
needs no headers, but not finding the host in the Servers table still
warned, so any install reached on a hostname the Servers table does not
list warned on ordinary use. Log that at Debug instead. Genuine
cross-origin requests still warn.
Headers are unchanged; this only affects logging and the empty-Origin
short circuit. The comparison ignores the scheme, so http:// against an
https-served HTTP_HOST counts as same-origin - that only suppresses a log
line, no header is emitted either way.
Checked with a standalone assert script over same-origin with and without
a port on both schemes, differing port, differing host, a suffix near-miss
(hamburg.local vs hamburg) and a missing HTTP_HOST; only the first three
go quiet.
Three points raised on #5038 after it was merged.
ajaxError() documents the reason field as included "only when set", but
tested it for truthiness, which would also drop '' and '0'. None of the
four STREAM_ERR_ constants are falsy so nothing changed behaviour, but
the check now matches the documented contract. The client already treats
an empty reason as fatal, so a caller that does pass one still gets the
old handling.
The case 0 branch called ajaxError() twice in sequence and relied on the
first one exiting to keep the second from running on a timeout. Made the
two paths mutually exclusive so it no longer depends on that.
The test's "every ajaxError call is classified" assertion compared the
number of call sites to the number of STREAM_ERR_ occurrences anywhere in
the file. The four define()s are part of that count, so up to four calls
could lose their classification and the test would still pass - verified
by dropping the argument from one call, which the old assertion accepted.
It now matches each call to the end of its statement and requires every
one to carry a constant.
getStreamCmdResponse() responded to every ajax/stream.php failure the same way:
mint a fresh connkey and reload the img src. ajaxError() returns HTTP 200 with
result=Error, so these arrive in jQuery's done() rather than fail(), and all
twelve error paths in stream.php took that branch.
Only one of them means zms is gone. For the rest the process is still running
and streaming, and replacing the connkey makes it unaddressable: CMD_STOP,
CMD_QUIT and mode=single all then go to the new key, so nothing can reach the
old process and only SIGPIPE can stop it, which we know is unreliable. That is
why the reports of lingering zms after switching monitors were unaffected by
changes to what the stop path sends.
The timeout path made this routine rather than rare. On select() expiry
ajaxError is commented out, so the script carries on to socket_recvfrom() on a
now non-blocking socket. That returns false, and false == 0 under switch's loose
comparison, so a merely slow zms was reported as 'No data to read from socket'
and torn down.
stream.php now classifies each failure as no_socket, timeout, transient or
invalid, and sends it as 'reason'. The client restarts the stream only for
no_socket. A missing reason is still treated as fatal, so a php that predates
this keeps the old behaviour.
Before replacing the connkey the client now sends CMD_QUIT to the old one, so
the process we are about to lose track of is asked to exit. That is deliberately
not routed through streamCommand(): it must name its target explicitly, since
this.connKey is about to change, and its response must not feed back into
getStreamCmdResponse(), or a QUIT that also failed would re-enter the error path
and loop.
ajaxError() takes the classification as a third argument, named $reason because
$code is already the HTTP status, and only includes it when set, so the other
131 callers are unaffected.
Tests: tests/js covers the fatal/non-fatal decision including the no-reason
fallback, tests/php pins the classification mapping and the switch(false)
semantics the timeout branch depends on. Both verified to fail when the
behaviour is reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
detaintPath() and detaintPathAllowAbsolute() removed '://' with a single
str_replace() while the '../' removal below them already looped. One
pass is not enough, because removing a match can join its neighbours
into a fresh match: '::////' collapses to '://'. So
'php::////filter/read=string.rot13/resource=/etc/passwd' came back out
of the filter as 'php://filter/read=string.rot13/resource=/etc/passwd',
reinstating exactly the wrapper the strip exists to remove.
Loop the '://' removal the same way the '../' removal is looped. These
functions guard $view, $request, $action, the modal name and skin file
paths.
Refs GHSA-wgqf-6fjf-7gxw.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
These functions are PHP 8.0+ but ZoneMinder is called from views on
PHP 7 installs (event.php uses str_ends_with at file scope, triggering
a fatal "Call to undefined function" before the page can render).
Add function_exists-guarded polyfills at the top of functions.php so
PHP 8+ keeps the native implementations and PHP 7.x picks up the
fallbacks. functions.php is required by index.php before any view, so
all four call sites (functions.php, event.php, views/image.php) are
covered.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous implementation used `str_replace($range, '-', $range)` which
is a no-op (wrong arg order, return value discarded), then cast the raw
Range value (e.g. "12345-67890" or "-500") to int. The function then
ignored the requested END entirely and always streamed from the parsed
start to EOF.
For a suffix range like `Range: bytes=-500` -- which Chrome's media stack
sends to locate the moov atom in many HEVC mp4s -- (int)"-500" is -500,
producing Content-Length = filesize + 500. fseek with SEEK_SET fails for
negative offsets, so the body delivered was filesize bytes against an
inflated Content-Length, triggering ERR_CONTENT_LENGTH_MISMATCH in the
browser and blocking HEVC playback in the files view.
Parse `bytes=start-end`, `bytes=start-`, and `bytes=-suffix` per RFC 7233,
clamp the end to file size, return 416 for unsatisfiable ranges, set
Content-Length to the actual byte count served, and stop reading once
that many bytes have been emitted. Guard ob_flush() with ob_get_level()
so it does not warn when no buffer is active.
Verified on pseudo by loading an HEVC mp4 in Chrome -- the
ERR_CONTENT_LENGTH_MISMATCH is gone, the browser parses metadata
(duration, dimensions) and buffers playback data normally.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The HTTP Content-Range header for partial content must use the form
"bytes start-end/total". output_file() was emitting "bytes startend/total"
with no separator, producing an invalid header that breaks range requests.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
FilterTerm.php:
- Use intval() on AlarmedZoneId value in SQL subquery to prevent
injection via crafted filter val
report_event_audit.php, montagereview.php:
- Cast $selected_monitor_ids through array_map('intval') before
interpolating into SQL IN clause (values come from $_REQUEST)
download_functions.php:
- Replace manual single-quoting with escapeshellarg() for merged
file name in ffmpeg, tar, and zip commands (monitor names can
contain shell metacharacters including single quotes)
- Same fix for export list file path
export_functions.php:
- Use escapeshellarg() on source and destination paths in cp -as
commands during event export
functions.php:
- Validate column keys in getFormChanges() against /^[a-zA-Z0-9_]+$/
to prevent SQL injection via crafted array keys from $_REQUEST
- Use dbEscape() and intval() for image/document MIME type and size
fields instead of raw string interpolation
- Replace escapeshellcmd() with escapeshellarg() in deletePath()
rm -rf command
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The zone loader now ignores the Units DB field and detects the coordinate
format by checking for decimal points: decimal values are percentages,
integer-only values are legacy pixels. This fixes motion detection being
broken when zones had Units=Pixels but percentage coordinates (or vice
versa), which resulted in a ~99x99 pixel zone on a 2560x1440 monitor.
The PHP zone view now always forces Units=Percent when saving, since it
always works in percentage space. convertPixelPointsToPercent() now
returns bool to indicate whether conversion occurred.
Tests added for: truncation bug via atoi, correct percentage-to-pixel
conversion, auto-detect heuristic, and resolution independence.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Device field from the Monitors table was interpolated directly into
shell commands (qx(), backticks, exec()) without sanitization, allowing
authenticated users with monitor-edit permissions to execute arbitrary
commands as www-data via the Device Path field.
Defense in depth:
- Input validation: reject Device values not matching /^\/dev\/[\w\/.\-]+$/
at save time in both web UI and REST API
- Output sanitization: use escapeshellarg() in PHP and quote validated
values in Perl at every shell execution point
Affected locations:
- scripts/ZoneMinder/lib/ZoneMinder/Monitor.pm (control, zmcControl)
- scripts/zmpkg.pl.in (system startup)
- web/includes/Monitor.php (zmcControl)
- web/includes/functions.php (zmcStatus, zmcCheck, validDevicePath)
- web/includes/actions/monitor.php (save action)
- web/api/app/Model/Monitor.php (daemonControl, validation rules)
- web/api/app/Controller/MonitorsController.php (daemonStatus)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When zone coordinates are stored as pixel values (e.g. from a missed DB
migration), the web layer now detects values > 100 and converts them to
percentages using the monitor's dimensions, mirroring the existing C++
detection logic in zm_zone.cpp. This prevents limitPoints() from clamping
pixel values to 0-100 and zones rendering incorrectly in SVG overlays.
- Add convertPixelPointsToPercent() helper in functions.php
- Call conversion before limitPoints() in zone.php and zones.php
- Update Zone::svg_polygon() to accept monitor dimensions and convert
- Pass ViewWidth/ViewHeight to svg_polygon() from Monitor::getStreamHTML()
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Convert zone coordinates from absolute pixel values to percentages
(0.00-100.00) so zones automatically adapt when monitor resolution
changes. This eliminates the need to manually reconfigure zones after
resolution adjustments.
Changes:
- Add DB migration (zm_update-1.37.81.sql) to convert existing pixel
coords to percentages, recalculate area, and update Units default
- Add Zone::ParsePercentagePolygon() in C++ to parse percentage coords
and convert to pixels at runtime using monitor dimensions
- Backwards compat: C++ Zone::Load() checks Units column and uses old
pixel parser for legacy 'Pixels' zones
- Update PHP coordsToPoints/mapCoords/getPolyArea for float coords,
replace scanline area algorithm with shoelace formula
- Update JS zone editor to work in percentage coordinate space with
SVG viewBox "0 0 100 100" and non-scaling-stroke for consistent
line thickness
- Position zone SVG overlay inside imageFeed container via JS to align
with image only (not status bar)
- Support array of zone IDs in Monitor::getStreamHTML zones option
- Update monitor resize handler: percentage coords don't need rescaling,
only threshold pixel counts are adjusted
- Add 8 Catch2 unit tests for ParsePercentagePolygon
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When adding cameras via ONVIF probe, passwords containing special
characters (like parentheses, slashes, etc.) were being stored in the
database in URL-encoded form instead of plain text. This caused
authentication failures when the encoded password was used.
The issue was in extract_auth_values_from_url() which extracted
credentials from the stream URI but didn't decode them. Since the ONVIF
probe process double-encodes passwords (to survive POST encoding), and
monitor.php decodes once, the extracted password still remained
URL-encoded.
The fix adds urldecode() to both username and password after extraction,
ensuring they're stored in their original form in the database.
Example: Password "pass)word" was being stored as "pass%29word"
