From 1e5329dd89d07c918a39b0b98af287d0949319dc Mon Sep 17 00:00:00 2001 From: TheWitness Date: Wed, 23 Sep 2026 09:45:42 -0400 Subject: [PATCH] security: consolidate verified GHSA-rjvj win32 cmd.exe hardening for develop (#8056) * fix(win32): harden the cmd.exe shell sinks on develop (GHSA-rjvj-r52f-8v5q) Forward-port of the 1.2.x win32 command-injection remediation. On Windows the SNMP/ping/data-query/trap commands run through cmd.exe, which ignores the \" escape and toggles quoting on every ", so cacti_escapeshellarg cannot stop & | ^ < > ( ). - add cacti_escapeshellarg_cmd(), stripping those operators on win32 before quoting (a Unix pass-through) - snmp_escape_string(): strip the operators in the win32 branch - route the SNMP binary hostname, the data-query device values (get_script_query_path), the data-input arg builders, the Windows ping hostname, and the SNMP trap notification arguments through the strip get_script_query_path is device-reachable (a rogue monitored device supplies an SNMP index/system field that executes on the Windows poller); the rest are post-auth. Signed-off-by: Thomas Vincent * fix(win32): strip %VAR% from the hostname on Windows (GHSA-rjvj-r52f-8v5q) cmd.exe expands %VAR% even inside quotes, so a device hostname of %USERNAME% would expand into the SNMP/ping command line. Add an opt-in $strip_env flag to cacti_escapeshellarg_cmd() and set it for the SNMP binary and ping hostnames, which never legitimately contain a percent. Community and credential values are left untouched because they can legitimately contain %. Signed-off-by: Thomas Vincent * docs(changelog): note win32 cmd.exe hardening for develop * security: require Package Management permission to trust new package signers package_accept_key() (reachable via the repository-based package import flow) inserted new keys into package_public_keys, the global trust store consulted by import_validate_signature() for every future import by every user, without checking for the Package Management (realm 29) permission that gates package_keys.php/package.php/package_repos.php for the same table. An Import Templates (realm 17) user could therefore add themselves as a trusted signer for the whole install. Also add the 'accept' action to the CSRF-protected bad_actions list so the acceptance step requires POST plus a valid token like every other state-changing action. GHSA-gxgm-8rhv-r7jv * docs(changelog): note package-signer trust authorization fix * security: close tab-based open redirect bypass in CSRF timeout handler sanitize_uri()'s guard (20298c6cd) models the WHATWG rule that browsers strip leading C0 controls/whitespace before resolving a redirect, but Chromium separately strips an embedded tab from anywhere in the target. A request target of /%09/evil.example passes the guard (no leading control char, not two leading slashes) but Chromium reduces the resulting 'Location: //evil.example' header to a network-path reference and navigates off-site. 1.2.x already uses validate_redirect_url() for this same sink (3338a63e1), which resolves the URL via parse_url() and checks host/port against the server-configured values, so it isn't bypassable this way. Switch develop's CSRF timeout handler to match. GHSA-c725-4jm5-9fx5 * security: authorize single-item Report actions against ownership reports.php sets \ = true, so include/auth.php grants an implicit guest session (when the Guest User feature is enabled) without ever evaluating the realm 22 gate declared for this page. The single-item action handlers reports_item_remove()/_moveup()/_movedown() relied solely on that page-level gate and performed no independent ownership check, so an unauthenticated guest-bound request could delete or reorder reports_items rows belonging to any report, including ones owned by an administrator. reports_form_actions() already gained an equivalent \ check; add the same authorization (Reports admin realm 21, or the requesting user owns the item's report) to the three single-item handlers. Same underlying chain as the 1.2.31 fix for reports_user.php (guest bind removed + cacti_authorize_resource() added to reports_item_remove()); the rewritten develop reports.php/lib/html_reports.php did not carry either half of that fix forward. GHSA-j5jh-fhwg-r56x * security: authorize tree node CRUD actions against per-tree access tree.php dispatches create_node/delete_node/move_node/rename_node/copy_node directly to api_tree_*() with no is_tree_allowed() check, even though the normal tree-view path already enforces that boundary. A non-admin user with the Trees realm but no explicit grant on a given tree_id could still create, delete, move, rename or copy nodes on that tree by calling these actions directly, bypassing the authorization boundary enforced elsewhere. GHSA-prxj-84qq-v5jq * security: remove shell from Realtime RRD create and validate DS bounds poller_realtime.php built the Realtime-graph RRD 'create' command line (including the SNMP-sourced ifSpeed/ifHighSpeed interface speed, substituted in verbatim as the DS maximum) and ran it through shell_exec(). A hostile or spoofed monitored device could return an ifSpeed value such as '1000000000\uid=197608(ladams) gid=197121 groups=197121' and get arbitrary OS command execution as the web/poller user the next time any user with the Realtime Graphs permission (or an unauthenticated guest, where configured) viewed a realtime graph for an affected data source. - lib/rrd.php: rrdtool_function_interface_speed() now casts ifHighSpeed/ ifSpeed to int before they become the DS maximum. - lib/rrd.php: rrdtool_function_create() now rejects any non-numeric rrd_minimum/rrd_maximum right before they are concatenated onto the rrdtool command line, forcing them to 'U' and logging instead. - poller_realtime.php: the RRD create is now routed through the same shell-free rrdtool_execute()/proc_open(bypass_shell) path every other RRDtool invocation already uses, matching the pattern 1.2.x already uses for this exact call site, instead of shell_exec(). GHSA-f88m-q789-g22j * security: authorize Report save/edit against report ownership reports_form_save()'s update path fetched the existing report's owner from the database but never compared it to the session user before calling sql_save(), so any user holding only the Reports realm could overwrite another user's report by POSTing its id. reports_edit() had the same gap on the read side: it fetched a report by id with no ownership check, so the same user could view any other user's report configuration (recipients, schedule, etc.). 1.2.x already guards both call sites with cacti_authorize_resource(..., 'reports'); develop had lost that check. GHSA-8f5c-3h59-q7gc * security: reject ORDER BY sort_column subquery bypass sanitize_sql_column()'s allowlist regex kept parentheses to support the INET_ATON(col)/NATURAL_SORT_KEY(col) sort wrappers, but that also let MySQL subquery syntax through unchanged, since a subquery like (SELECT(0)FROM(SELECT(SLEEP(5)))a) needs only alphanumerics and parentheses. get_order_string() then detects the '(' and disables its backtick quoting, so the payload reaches ORDER BY verbatim, giving any user with realm 2-5 a blind, time-based SQL injection via sort_column. sanitize_sql_column() now only allows '(' through when the whole value is exactly one of the two known safe wrapper forms around a plain identifier; anything else containing '(' -- including subquery syntax -- now falls back to the default. Added regression tests covering the reported subquery and REGEXP-exfiltration payloads. GHSA-8j73-cvf6-jrvx * security: confine Reports format_file to a bare filename reports_form_save() wrote the POST-supplied format_file value verbatim into reports.format_file, and reports_load_format_file() concatenated it onto CACTI_PATH_FORMATS with no directory-component check, so a value such as ../../../../etc/passwd escaped the formats directory and any Reports-realm user could read arbitrary files readable by the web server. 1.2.x already fixed the same sink (GHSA-g37j-39f4-6r4j / CVE-2026-41884, GHSA-mjvw-mhj5-9jcj / CVE-2026-40084); develop had lost the guard. format_file is now confined to basename() at both the save site and the load site, with the load site additionally rejecting a bare '.'/'..'/empty result before any file IO. Added regression tests. GHSA-5x27-wqpm-m24v * security: authorize Data Source enable/disable against owning device ds_enable()/ds_disable() and the bulk drp_action enable/disable loops in form_actions() passed a user-controlled local_data_id straight into api_data_source_enable()/api_data_source_disable() with no object-level authorization check, so any user holding the Sites/Devices/Data realm could toggle the enabled state of a data source belonging to a device outside their assigned Device Permissions. Added data_source_authorized(\), which allows host-independent data sources (data_local.host_id = 0) and otherwise defers to is_device_allowed(\), and applied it to both the single-item and bulk action paths. GHSA-2fhv-c794-v433 * security: authorize bulk Change Data Source Profile against owning device The bulk 'Change Data Source Profile' action (drp_action == '6') iterated selected_items and updated data_template_data/data_template_rrd/poller_item for each supplied local_data_id with no object-level authorization check, so any Sites/Devices/Data realm user could retarget the RRD step/heartbeat of a data source belonging to a device outside their assigned Device Permissions -- the same class of gap as GHSA-2fhv, in a different bulk action. Guarded with the data_source_authorized() helper added for that fix. This action does not exist on 1.2.x (develop-only feature). GHSA-fc82-w6xq-4jg7 * security: grant AllowOverride All in docker/apache.conf The packaged docker/apache.conf granted only 'AllowOverride FileInfo Options=FollowSymLinks' for the Cacti document root. Cacti's shipped .htaccess deny rules (log/, cache/*, scripts/, cli/, bin/, mibs/, rra/, contrib/, tests/) rely on 'Require all denied' (Apache 2.4, override type AuthConfig) and/or legacy 'Order/Deny' (override type Limit), neither of which FileInfo/Options covers, so Apache silently ignored every one of those deny rules and served the directories over HTTP to any unauthenticated client -- most notably log/cacti.log, which can contain usernames, client IPs, SQL backtraces, and any secrets Cacti logs (SNMP community strings, LDAP bind details). AllowOverride All lets every shipped .htaccess apply as intended. Added a regression test. docker/ does not exist on 1.2.x (develop-only). GHSA-fxfg-j995-j69j * docs: add security advisory / CVE policy to Copilot instructions Documents the policy established for this remediation effort: develop is unreleased, so a develop-only vulnerability does not get a CVE requested; 1.2.x is the released/LTS branch, so a CVE is requested when the same vulnerable code is also reachable there. Also notes to verify against both branches' actual source rather than trusting a reporter's claimed affected-version range. * fix(security): update stale test harnesses and address review findings CI fixes: - CsrfCheckContractProbe.php: stub validate_redirect_url() so the fatal-fail probe doesn't fatal on the new open-redirect dependency that csrf_error_callback() picked up. - InstallerCsrfRecoveryTest.php: update the stale sanitize_uri()-based assertion to match the new validate_redirect_url() redirect code. - psalm-baseline.xml: drop the TaintedHeader suppression for include/csrf.php now that validate_redirect_url() resolves it. Copilot review findings on GHSA-rjvj-r52f-8v5q hardening: - lib/data_query.php: only strip '%' for the host_hostname/ host_management_ip substitutions in get_script_query_path(), not indiscriminately for credential substitutions. - lib/functions.php: opt into percent stripping for the hostname host column in get_full_test_script_path() and for type_code='hostname' fields in get_full_script_path(). - lib/snmpagent.php: sanitize the trap receiver hostname against percent expansion before it reaches the escaping helper. - tests/security/baselines/*.tsv: refresh the sink inventory and architectural hotspot baselines for the updated command sinks. - tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php: add the missing cacti_escapeshellarg_cmd stub, and update the snmp_escape_string contract test for the intentional win32 quote-stripping hardening. - tests/Unit/Security/Shell/CmdMetacharHelperTest.php + tests/fixtures/CmdMetacharHelperWin32Probe.php: add a real win32 subprocess execution test for cacti_escapeshellarg_cmd instead of only asserting on source text. - CHANGELOG: add the pull-request reference to the new entry. * fix(snmp): remove now-unreachable quote-escaping branch in snmp_escape_string The metachar strip on the line above already removes the double quote on win32, so the substr_count(SNMP_ESCAPE_CHARACTER) branch below it could never execute; drop it so lib/snmp.php reaches 100% coverage again. * style: fix pre-existing php-cs-fixer blank-line drift blocking CI data_sources.php and lib/html_reports.php had a pre-existing blank-line style drift (unrelated to this PR's security fixes) that php-cs-fixer's --dry-run check flags as fixable. Since "PHP 8.3 locked integration" is part of this PR's CI / required gate, correct the two spots so the whole PR is green: - data_sources.php: collapse a double blank line before form_actions(). - lib/html_reports.php: insert the blank line php-cs-fixer expects between the input-validation gfrv() calls and the following "====" separator comment in reports_item_moveup() and reports_item_remove(). No behavior change; purely CI/style formatting. * style: refresh phpstan baseline for pre-existing, unrelated findings Three PHPStan findings were masked behind the earlier php-cs-fixer failure and only surfaced once that step passed: - lib/html_reports.php: a second, already-guarded 'user_id' offset access from an unrelated, previously-merged security commit (92b8d4e6f) was never added to the baseline count. - cli/splice_rrd.php, poller_realtime.php: two other pre-existing, unrelated findings had no baseline entry at all. None of these touch code from this PR; added narrowly-scoped baseline entries (not a full regeneration) purely to unblock the required PHP 8.3 locked integration check. * fix(security): address follow-up review findings on consolidated PR Closes the four remaining review findings from the follow-up review round: - package_import.php: trustSigner() now POSTs with a CSRF token instead of $.getJSON, since 'accept' is enforced as a POST-only action in include/global.php's bad_actions list (GHSA-gxgm-8rhv-r7jv). - lib/html_reports.php: reports_item_authorized() gained an optional $report_id parameter that, when given, requires it to match the item's actual current report - reports_item_movedown()/reports_item_moveup() now pass the request's report id through it, closing a cross-report sequence-swap variant where an item you own could be reordered against a report you don't. The item_edit render path and the save_component_report_item save path (reachable via reports.php's guest bind) previously had no ownership check at all; both now verify the target report is owned by the caller, and (when editing an existing item) that the item's current report is too, before rendering or calling sql_save() (GHSA-j5jh-fhwg-r56x). - tree.php + include/global.php: create_node/rename_node/move_node/copy_node now submit as POST with a CSRF token (matching the existing delete_node pattern) and are added to the POST-only bad_actions list, since the per-tree authorization check added earlier didn't make these mutations CSRF-safe on its own (GHSA-prxj-84qq-v5jq). Also updates the PR description to list every GHSA fix actually included in this consolidated branch (previously only GHSA-rjvj-r52f-8v5q was mentioned), per review feedback that the description didn't match the actual scope. * fix: resolve CI failures on PR #8056 (docblock alignment, sink baseline) - lib/html_reports.php: fix php-cs-fixer docblock indentation drift on reports_item_authorized()'s @param $report_id continuation lines (one extra space of alignment) introduced by the prior ownership-check fix. - tests/security/baselines/sink_inventory.baseline.tsv: refresh to track the 4 new header('Location: ' . get_reports_page()) redirect sinks added by the new report/report-item ownership checks in lib/html_reports.php. Verified via tests/security/verify_sink_inventory.sh and confirmed tests/security/verify_architectural_hotspots.sh still reports no new hotspots. * fix: track new PHPStan offset findings from extended reports_item_authorized() phpstan-baseline.neon: the reports_item_authorized($item_id, $report_id) extension added a new array offset access ('report_id') and a third occurrence of the existing 'user_id' offset access on the array|bool return of db_fetch_row_prepared() in lib/html_reports.php. Add a new baseline entry for 'report_id' (count: 1) and bump the existing 'user_id' entry from count: 2 to count: 3 to match. Verified locally with `include/vendor/bin/phpstan analyse --memory-limit=1G` (0 errors). --------- Signed-off-by: Thomas Vincent Co-authored-by: Thomas Vincent Co-authored-by: TheWitness --- .github/copilot-instructions.md | 5 + CHANGELOG | 12 ++ data_sources.php | 49 +++++- docker/apache.conf | 7 +- include/csrf.php | 12 +- include/global.php | 4 +- lib/data_query.php | 20 ++- lib/functions.php | 65 ++++++- lib/html_reports.php | 125 ++++++++++++- lib/ping.php | 24 +-- lib/reports.php | 9 + lib/rrd.php | 14 +- lib/snmp.php | 23 +-- lib/snmpagent.php | 12 +- package_import.php | 22 ++- phpstan-baseline.neon | 26 ++- poller_realtime.php | 10 +- psalm-baseline.xml | 5 - .../DataCollection/Snmp/SnmpCoverageTest.php | 14 +- .../Auth/DataSourceEnableDisableAuthzTest.php | 84 +++++++++ .../Csrf/InstallerCsrfRecoveryTest.php | 3 +- .../Misc/DockerApacheHtaccessOverrideTest.php | 36 ++++ .../ReportsFormatFileTraversalTest.php | 57 ++++++ .../Security/Shell/CmdMetacharHelperTest.php | 134 ++++++++++++++ .../Security/SqlInjection/SafeSortTest.php | 37 +++- .../fixtures/CmdMetacharHelperWin32Probe.php | 43 +++++ tests/fixtures/CsrfCheckContractProbe.php | 8 + .../architectural_hotspots.baseline.tsv | 165 ++++++++++++++++-- .../baselines/sink_inventory.baseline.tsv | 42 +++-- tree.php | 43 ++++- 30 files changed, 1014 insertions(+), 96 deletions(-) create mode 100644 tests/Unit/Security/Auth/DataSourceEnableDisableAuthzTest.php create mode 100644 tests/Unit/Security/Misc/DockerApacheHtaccessOverrideTest.php create mode 100644 tests/Unit/Security/PathTraversal/ReportsFormatFileTraversalTest.php create mode 100644 tests/Unit/Security/Shell/CmdMetacharHelperTest.php create mode 100644 tests/fixtures/CmdMetacharHelperWin32Probe.php diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index fbd2551c3..6420f9598 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -104,6 +104,11 @@ - **CSRF**: Verify forms/AJAX include `__csrf_magic`. - **Command Injection**: Scrutinize `shell_exec`, `exec`, `passthru`. Ensure arguments are escaped (`escapeshellarg`). - **Auth**: Verify `include/auth.php` is loaded and permissions are checked (`api_user_realm_auth`). +- **Security advisories and CVE policy**: + - `develop`/`1.3.x` is unreleased. A vulnerability that exists **only** on `develop` does **not** get a CVE requested, since no released version is affected — document that determination in the GHSA description (a dated "Triage Note") instead. + - `1.2.x` is the released/LTS branch. If the same vulnerable code is also reachable on `1.2.x` (or was historically, for an unpatched released version), **a CVE is requested**. + - Before deciding CVE vs. no-CVE, verify the claim against both branches' actual current source rather than trusting the reporter's stated affected-version range — it can be wrong (e.g. describing a bug already fixed on `1.2.x` that only regressed on `develop`). + - If a `develop`-only advisory is later found to also affect a released `1.2.x` version, request a CVE at that point. ## Clean-as-you-code - While implementing a change, do small, behavior-preserving refactors in the *touched area* (dedupe logic, extract helpers, simplify conditions). diff --git a/CHANGELOG b/CHANGELOG index f86b77e6e..2962ceeda 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,18 @@ Cacti CHANGELOG 1.3.0-dev +-security: Grant AllowOverride All in docker/apache.conf so shipped .htaccess denies on log/cache/scripts/cli take effect (GHSA-fxfg-j995-j69j) +-security: Authorize the bulk Change Data Source Profile action against the owning device (GHSA-fc82-w6xq-4jg7) +-security: Authorize Data Source enable/disable against the owning device before mutating (GHSA-2fhv-c794-v433) +-security: Confine Reports format_file to a bare filename to close a path traversal/LFI regression (GHSA-5x27-wqpm-m24v) +-security: Reject ORDER BY sort_column subquery payloads that bypassed the GHSA-q9xg-p762-9jm3 fix (GHSA-8j73-cvf6-jrvx) +-security: Authorize Report save/edit against report ownership to close an authenticated IDOR (GHSA-8f5c-3h59-q7gc) +-security: Stop building the Realtime RRD create command through a shell and validate DS bounds are numeric before they reach it (GHSA-f88m-q789-g22j) +-security: Authorize tree node create/delete/move/rename/copy against per-tree access before mutating (GHSA-prxj-84qq-v5jq) +-security: Authorize single-item Report actions (remove/move) against report ownership to close a guest-bind IDOR (GHSA-j5jh-fhwg-r56x) +-security: Use validate_redirect_url() for the CSRF timeout redirect to close a tab-based open redirect bypass (GHSA-c725-4jm5-9fx5) +-security: Require Package Management permission to trust new package signers (GHSA-gxgm-8rhv-r7jv) +-security: Harden cmd.exe shell sinks and strip %VAR% from hostnames on win32 (GHSA-rjvj-r52f-8v5q) (#8056) -security: Reject SHA-1 signatures across every package import path -security: Escape path_dsstats_log before it reaches the dsstats poller shell redirect -security: Revoke anonymous access when the guest account is disabled diff --git a/data_sources.php b/data_sources.php index 76ad09fd5..dc2689b90 100644 --- a/data_sources.php +++ b/data_sources.php @@ -379,6 +379,25 @@ function form_save() : void { } } +/** + * Determines whether the current user is authorized to modify the given data source. + * Data sources tied to a device (host_id > 0) require the caller to be authorized for + * that device; host-independent data sources (host_id = 0) are not device-scoped. + * + * @param int $local_data_id The data source to check. + * + * @return bool True if the caller may modify this data source. + */ +function data_source_authorized(int $local_data_id) : bool { + $host_id = db_fetch_cell_prepared('SELECT host_id FROM data_local WHERE id = ?', [$local_data_id]); + + if (empty($host_id)) { + return true; + } + + return is_device_allowed((int) $host_id); +} + function form_actions() : void { global $actions; @@ -450,12 +469,16 @@ function form_actions() : void { api_data_source_remove_multi($selected_items); } elseif (gnrv('drp_action') == '2') { // data source disable - for ($i = 0; ($i < cacti_count($selected_items)); $i++) { - api_data_source_disable($selected_items[$i]); + foreach ($selected_items as $local_data_id) { + if (data_source_authorized((int) $local_data_id)) { + api_data_source_disable($local_data_id); + } } } elseif (gnrv('drp_action') == '3') { // data source enable - for ($i = 0; ($i < cacti_count($selected_items)); $i++) { - api_data_source_enable($selected_items[$i]); + foreach ($selected_items as $local_data_id) { + if (data_source_authorized((int) $local_data_id)) { + api_data_source_enable($local_data_id); + } } } elseif (gnrv('drp_action') == '4') { // change host gfrv('host_id'); @@ -477,6 +500,10 @@ function form_actions() : void { $remote_conns = []; foreach ($selected_items as $local_data_id) { + if (!data_source_authorized((int) $local_data_id)) { + continue; + } + $data = db_fetch_row_prepared('SELECT dl.host_id, h.poller_id FROM host AS h INNER JOIN data_local AS dl @@ -878,6 +905,13 @@ function ds_disable() : void { gfrv('id'); // ==================================================== + if (!data_source_authorized((int) grv('id'))) { + raise_message('permission_denied'); + header('Location: data_sources.php'); + + return; + } + api_data_source_disable(grv('id')); header('Location: data_sources.php?action=ds_edit&id=' . grv('id')); } @@ -887,6 +921,13 @@ function ds_enable() : void { gfrv('id'); // ==================================================== + if (!data_source_authorized((int) grv('id'))) { + raise_message('permission_denied'); + header('Location: data_sources.php'); + + return; + } + api_data_source_enable(grv('id')); header('Location: data_sources.php?action=ds_edit&id=' . grv('id')); } diff --git a/docker/apache.conf b/docker/apache.conf index 42785276a..6e973c550 100644 --- a/docker/apache.conf +++ b/docker/apache.conf @@ -4,7 +4,12 @@ Options -Indexes +FollowSymLinks - AllowOverride FileInfo Options=FollowSymLinks + # Cacti ships .htaccess deny rules in log/, include/, scripts/, cache/, + # and cli/ (Require all denied / Order,Deny) to block direct web access + # to logs and non-executed files. FileInfo/Options alone does not cover + # the AuthConfig/Limit override types those rules need, which silently + # disables them (GHSA-fxfg-j995-j69j); AllowOverride All lets them apply. + AllowOverride All Require all granted # Proxy PHP requests to php-fpm diff --git a/include/csrf.php b/include/csrf.php index 1a5df43d1..957b79cfb 100644 --- a/include/csrf.php +++ b/include/csrf.php @@ -401,8 +401,16 @@ function csrf_error_callback() : void { raise_message('csrf_timeout'); ob_end_clean(); - header('Location: ' . sanitize_uri($_SERVER['REQUEST_URI'])); - csrf_log(__FUNCTION__, 'Timeout, redirecting to ' . sanitize_uri($_SERVER['REQUEST_URI'])); + // sanitize_uri()'s guard models the WHATWG C0/leading-slash normalisation + // rule but not Chromium's separate stripping of embedded tabs, so a + // target like "/%09/evil.example" still reaches the browser as a + // same-origin-looking "//evil.example" once Chromium collapses the tab. + // validate_redirect_url() resolves the URL with parse_url() and compares + // host/port against the server-configured values instead of pattern + // matching known-bad prefixes, so it is not bypassable this way. + $redirect_target = validate_redirect_url($_SERVER['REQUEST_URI'] ?? '', 'index.php'); + header('Location: ' . $redirect_target); + csrf_log(__FUNCTION__, 'Timeout, redirecting to ' . $redirect_target); exit; } diff --git a/include/global.php b/include/global.php index 0b92c47a3..e944bd756 100644 --- a/include/global.php +++ b/include/global.php @@ -809,9 +809,11 @@ if ($config['is_web']) { 'item_remove_gsv', 'item_remove_dssv', 'item_moveup_gsv', 'item_moveup_dssv', 'item_movedown_gsv', 'item_movedown_dssv', - 'delete_node', 'gt_remove', 'query_remove', 'remove', 'change_leaf', + 'delete_node', 'create_node', 'rename_node', 'move_node', 'copy_node', + 'gt_remove', 'query_remove', 'remove', 'change_leaf', 'moveup', 'movedown', 'tree_up', 'tree_down', 'move_page_up', 'move_page_down', 'rrd_add', 'rrd_remove', + 'accept', ]; foreach ($bad_actions as $bad) { diff --git a/lib/data_query.php b/lib/data_query.php index ad31e04b1..de5a2c558 100644 --- a/lib/data_query.php +++ b/lib/data_query.php @@ -2800,7 +2800,25 @@ function get_script_query_path(string $args, string $script_path, int $host_id) $extra_arguments = ''; foreach ($parts as $index => $part) { - $extra_arguments .= ($index > 0 ? ' ' : '') . cacti_escapeshellarg(substitute_host_data($part, '|', '|', $host_id)); + /* only the hostname/IP placeholders are stripped of '%'; SNMP + * community and credential substitutions may legitimately + * contain one, so they keep their value unmodified. */ + $strip_env = (strpos($part, '|host_hostname|') !== false || strpos($part, '|host_management_ip|') !== false); + + $part = substitute_host_data($part, '|', '|', $host_id); + + /* GHSA-rjvj-r52f-8v5q: a rogue device's SNMP index or system field + * reaches here; on Windows cmd.exe would interpret & | ^ < > ( ) it + * carries, so strip them before quoting. */ + if (CACTI_SERVER_OS == 'win32') { + $part = str_replace(['"', '&', '|', '^', '<', '>', '(', ')'], '', $part); + + if ($strip_env) { + $part = str_replace('%', '', $part); + } + } + + $extra_arguments .= ($index > 0 ? ' ' : '') . cacti_escapeshellarg($part); } } else { $extra_arguments = ''; diff --git a/lib/functions.php b/lib/functions.php index dc83ab77c..989d3c882 100644 --- a/lib/functions.php +++ b/lib/functions.php @@ -3022,11 +3022,14 @@ function get_full_test_script_path(int $data_template_id, int $host_id) : mixed if (cacti_sizeof($data) && is_array($host)) { foreach ($data as $item) { if (isset($host[$item['data_name']])) { - $value = cacti_escapeshellarg($host[$item['data_name']]); + /* only the hostname column may legitimately need percent + * stripping on Windows; other host columns (e.g. SNMP + * community/credentials) can contain a literal '%'. */ + $value = cacti_escapeshellarg_cmd($host[$item['data_name']], true, $item['data_name'] === 'hostname'); } elseif ($item['data_name'] == 'host_id' || $item['data_name'] == 'hostid') { $value = cacti_escapeshellarg($host['id']); } else { - $value = cacti_escapeshellarg((string) $item['value']); + $value = cacti_escapeshellarg_cmd((string) $item['value']); } $full_path = str_replace('<' . $item['data_name'] . '>', $value, $full_path); @@ -3076,7 +3079,7 @@ function get_full_script_path(int $local_data_id) : mixed { return false; } - $data = db_fetch_assoc_prepared('SELECT ' . SQL_NO_CACHE . " dif.data_name, did.value + $data = db_fetch_assoc_prepared('SELECT ' . SQL_NO_CACHE . " dif.data_name, dif.type_code, did.value FROM data_input_fields AS dif LEFT JOIN data_input_data AS did ON dif.id = did.data_input_field_id @@ -3089,7 +3092,10 @@ function get_full_script_path(int $local_data_id) : mixed { if (cacti_sizeof($data)) { foreach ($data as $item) { - $value = cacti_escapeshellarg($item['value']); + /* only hostname-class fields may legitimately need percent + * stripping on Windows; other input fields (e.g. SNMP + * community/credentials) can contain a literal '%'. */ + $value = cacti_escapeshellarg_cmd($item['value'], true, $item['type_code'] === 'hostname'); if ($value == '') { $value = "''"; @@ -5214,9 +5220,14 @@ function cacti_redirect(string $path, array $params = []) : never { /** * Strips any character that cannot safely appear in a SQL identifier. - * Allows word characters, dots (table.column), parentheses and INET_ATON-style - * wrappers already used by the sort helpers. Use before concatenating a - * user-supplied sort column into an ORDER BY clause. + * Allows word characters and dots (table.column). Parentheses are only + * permitted when the entire value is one of the known safe function-call + * wrappers (INET_ATON(col), NATURAL_SORT_KEY(col)) around a plain identifier; + * any other use of '(' -- including MySQL subquery syntax, which needs only + * alphanumerics and parentheses -- is rejected back to the default rather + * than passed through, since a bare character-class allowlist alone cannot + * distinguish a function wrapper from a subquery. + * Use before concatenating a user-supplied sort column into an ORDER BY clause. * * @param string $column Raw sort-column value from user input. * @param string $default Fallback returned when all characters are stripped (default 'id'). @@ -5226,7 +5237,15 @@ function cacti_redirect(string $path, array $params = []) : never { function sanitize_sql_column(string $column, string $default = 'id') : string { $result = preg_replace('/[^a-zA-Z0-9_().]/', '', $column) ?? ''; - return $result !== '' ? $result : $default; + if ($result === '') { + return $default; + } + + if (str_contains($result, '(') && preg_match('/^(?:INET_ATON|NATURAL_SORT_KEY)\([a-zA-Z_][a-zA-Z0-9_.]*\)$/i', $result) !== 1) { + return $default; + } + + return $result; } /** @@ -5574,6 +5593,36 @@ function cacti_escapeshellarg(string $string, bool $quote = true) : string { } } +/** + * cacti_escapeshellarg_cmd - escape an argument that will reach cmd.exe on Windows. + * + * On Windows, exec()/shell_exec()/popen() route through cmd.exe, which ignores + * the \" escape and toggles quote-state on every ", so cacti_escapeshellarg() + * alone cannot stop the operators & | ^ < > ( ). Device- and request-supplied + * values that reach a Windows shell never legitimately contain these, so strip + * them before quoting. On Unix this is exactly cacti_escapeshellarg(). + * GHSA-rjvj-r52f-8v5q. + * + * @param string $string The value to place in a Windows command line. + * @param bool $quote Whether to wrap the result in quotes. + * + * @return string The escaped value. + */ +function cacti_escapeshellarg_cmd(string $string, bool $quote = true, bool $strip_env = false) : string { + if (CACTI_SERVER_OS == 'win32') { + $string = str_replace(['"', '&', '|', '^', '<', '>', '(', ')'], '', $string); + + /* cmd.exe expands %VAR% even inside quotes. Only values that never + * legitimately contain a percent (a hostname or IP) may strip it; SNMP + * community and credential values can contain %, so callers opt in. */ + if ($strip_env) { + $string = str_replace('%', '', $string); + } + } + + return cacti_escapeshellarg($string, $quote); +} + /** * set a page refresh in Cacti through a callback * diff --git a/lib/html_reports.php b/lib/html_reports.php index eb4a80f17..74f752ede 100644 --- a/lib/html_reports.php +++ b/lib/html_reports.php @@ -272,6 +272,13 @@ function reports_form_save() : void { $save['user_id'] = $_SESSION[SESS_USER_ID]; } else { $save['user_id'] = db_fetch_cell_prepared('SELECT user_id FROM reports WHERE id = ?', [$post['id']]); + + if (!is_reports_admin() && (int) $save['user_id'] !== (int) $_SESSION[SESS_USER_ID]) { + raise_message('reports_idor', __('You do not have permission to modify this Report.'), MESSAGE_LEVEL_ERROR); + header('Location: ' . get_reports_page()); + + exit; + } } $save['id'] = $post['id']; @@ -280,7 +287,9 @@ function reports_form_save() : void { $save['enabled'] = (isset($post['enabled']) ? 'on' : ''); $save['cformat'] = (isset($post['cformat']) ? 'on' : ''); - $save['format_file'] = $post['format_file']; + // format files are flat files under CACTI_PATH_FORMATS; confine the + // stored value to a bare filename so it cannot carry a path traversal. + $save['format_file'] = basename((string) ($post['format_file'] ?? '')); $save['font_size'] = form_input_validate($post['font_size'], 'font_size', '^[0-9]+$', false, 3); $save['alignment'] = form_input_validate($post['alignment'], 'alignment', '^[0-9]+$', false, 3); $save['graph_linked'] = (isset($post['graph_linked']) ? 'on' : ''); @@ -344,6 +353,26 @@ function reports_form_save() : void { $save['id'] = gnrv('id'); $save['report_id'] = form_input_validate(gnrv('report_id'), 'report_id', '^[0-9]+$', false, 3); + /* reports.php's guest bind makes this reachable without the page-level + * realm check (see reports_item_authorized()) - independently verify the + * caller owns both the target report and, when editing, the item's + * current report before touching reports_items */ + $report_owner = db_fetch_cell_prepared('SELECT user_id FROM reports WHERE id = ?', [$save['report_id']]); + + if (!is_reports_admin() && (int) $report_owner !== (int) $_SESSION[SESS_USER_ID]) { + raise_message('reports_idor', __('You do not have permission to modify this Report.'), MESSAGE_LEVEL_ERROR); + header('Location: ' . get_reports_page()); + + exit; + } + + if ((int) $save['id'] > 0 && !reports_item_authorized((int) $save['id'])) { + raise_message('reports_idor', __('You do not have permission to modify this Report Item.'), MESSAGE_LEVEL_ERROR); + header('Location: ' . get_reports_page()); + + exit; + } + if (ierv('id')) { $save['sequence'] = db_fetch_cell_prepared('SELECT MAX(sequence)+1 FROM reports_items @@ -677,6 +706,57 @@ function reports_send(int $id) : void { } } +/** + * Determines whether the current session user may modify a report item. + * + * reports.php sets $guest_account = true, so include/auth.php grants an + * implicit guest session (when the Guest User feature is enabled) without + * ever evaluating the realm 22 gate that user_auth_realm_filenames declares + * for this page. The single-item action handlers below are reachable from + * that guest bind, so they must not rely on the page-level realm check and + * must independently verify the caller is either a Reports administrator or + * the owning user before mutating a reports_items row. + * + * @param int $item_id The reports_items.id to authorize against. + * @param int|null $report_id When provided, also require that this is the + * item's actual current report_id - callers that + * take a separate report/id request parameter + * (e.g. the move-up/move-down and edit/save + * handlers) must not let it diverge from the + * item's real report and be used to scope + * operations against a report the item isn't + * even in. + * + * @return bool True when the current user may modify the item's report. + */ +function reports_item_authorized(int $item_id, ?int $report_id = null) : bool { + if (empty($_SESSION[SESS_USER_ID])) { + return false; + } + + $item = db_fetch_row_prepared('SELECT ri.report_id, r.user_id + FROM reports_items AS ri + INNER JOIN reports AS r + ON r.id = ri.report_id + WHERE ri.id = ?', + [$item_id] + ); + + if (!cacti_sizeof($item)) { + return false; + } + + if ($report_id !== null && (int) $item['report_id'] !== $report_id) { + return false; + } + + if (is_reports_admin()) { + return true; + } + + return ((int) $item['user_id'] === (int) $_SESSION[SESS_USER_ID]); +} + /** * Moves a report item down in the order. * @@ -691,6 +771,12 @@ function reports_item_movedown() : void { gfrv('id'); // ==================================================== + if (!reports_item_authorized((int) grv('item_id'), (int) grv('id'))) { + raise_message('permission_denied'); + + return; + } + move_item_down('reports_items', grv('item_id'), 'report_id=' . grv('id')); } @@ -707,7 +793,14 @@ function reports_item_moveup() : void { // ================= input validation ================= gfrv('item_id'); gfrv('id'); + // ==================================================== + if (!reports_item_authorized((int) grv('item_id'), (int) grv('id'))) { + raise_message('permission_denied'); + + return; + } + move_item_up('reports_items', grv('item_id'), 'report_id=' . grv('id')); } @@ -722,7 +815,14 @@ function reports_item_moveup() : void { function reports_item_remove() : void { // ================= input validation ================= gfrv('item_id'); + // ==================================================== + if (!reports_item_authorized((int) grv('item_id'))) { + raise_message('permission_denied'); + + return; + } + db_execute_prepared('DELETE FROM reports_items WHERE id = ?', [grv('item_id')]); } @@ -991,11 +1091,27 @@ function reports_item_edit() : void { $report_item['tree_id'] = -1; if (isrv('item_id') && gfrv('item_id') > 0) { + if (!reports_item_authorized((int) grv('item_id'))) { + raise_message('reports_idor', __('You do not have permission to view this Report Item.'), MESSAGE_LEVEL_ERROR); + header('Location: ' . get_reports_page()); + + exit; + } + $report_item = db_fetch_row_prepared('SELECT * FROM reports_items WHERE id = ?', [grv('item_id')] ); } else { + $report_owner = db_fetch_cell_prepared('SELECT user_id FROM reports WHERE id = ?', [grv('id')]); + + if (!is_reports_admin() && (int) $report_owner !== (int) $_SESSION[SESS_USER_ID]) { + raise_message('reports_idor', __('You do not have permission to view this Report.'), MESSAGE_LEVEL_ERROR); + header('Location: ' . get_reports_page()); + + exit; + } + $report_item['report_id'] = grv('id'); $report_item['local_graph_id'] = 0; @@ -1578,6 +1694,13 @@ function reports_edit() : void { if (gfrv('id') > 0) { $report = db_fetch_row_prepared('SELECT * FROM reports WHERE id = ?', [grv('id')]); + + if (cacti_sizeof($report) && !is_reports_admin() && (int) $report['user_id'] !== (int) $_SESSION[SESS_USER_ID]) { + raise_message('reports_idor', __('You do not have permission to view this Report.'), MESSAGE_LEVEL_ERROR); + header('Location: ' . get_reports_page()); + + exit; + } } reports_tabs(grv('id')); diff --git a/lib/ping.php b/lib/ping.php index 818cbdf9f..37c15de9f 100644 --- a/lib/ping.php +++ b/lib/ping.php @@ -172,7 +172,7 @@ class Net_Ping { } } - $result = shell_exec(cacti_escapeshellarg($fping) . ' -q -t ' . $this->timeout . ' -c 1 -r ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname']) . ' 2>&1'); + $result = shell_exec(cacti_escapeshellarg($fping) . ' -q -t ' . $this->timeout . ' -c 1 -r ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true) . ' 2>&1'); } else { $using_fping = false; @@ -183,25 +183,25 @@ class Net_Ping { * DNS resolution check above. */ if (substr_count(cacti_strtolower(PHP_OS), 'sun')) { - $result = shell_exec('ping ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } elseif (substr_count(cacti_strtolower(PHP_OS), 'hpux')) { - $result = shell_exec('ping -m ' . ceil($this->timeout / 1000) . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -m ' . ceil($this->timeout / 1000) . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } elseif (substr_count(cacti_strtolower(PHP_OS), 'mac')) { - $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } elseif (substr_count(cacti_strtolower(PHP_OS), 'freebsd')) { if (filter_var($host_ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { - $result = shell_exec('ping6 -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping6 -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } else { - $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } } elseif (substr_count(cacti_strtolower(PHP_OS), 'darwin')) { - $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } elseif (substr_count(cacti_strtolower(PHP_OS), 'bsd')) { - $result = shell_exec('ping -w ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -w ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } elseif (substr_count(cacti_strtolower(PHP_OS), 'aix')) { - $result = shell_exec('ping -i ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -i ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } elseif (cacti_strtoupper(substr(PHP_OS, 0, 3)) === 'WIN') { - $result = shell_exec('chcp 437 && ping -w ' . $this->timeout . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('chcp 437 && ping -w ' . $this->timeout . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } else { /** * Please know, that when running SELinux, httpd will throw @@ -210,9 +210,9 @@ class Net_Ping { * $result will be empty, then. */ if (filter_var($host_ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) { - $result = shell_exec('ping -6 -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg($this->host['hostname'])); + $result = shell_exec('ping -6 -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); } else { - $result = shell_exec('ping -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg($this->host['hostname']) . ' 2>&1'); + $result = shell_exec('ping -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true) . ' 2>&1'); } } } diff --git a/lib/reports.php b/lib/reports.php index 1358db1b4..6d3438abf 100644 --- a/lib/reports.php +++ b/lib/reports.php @@ -693,6 +693,15 @@ function reports_load_format_file(string $format_file, mixed &$output, bool &$re $format_file = 'cacti_group.format'; } + // format files are always flat files directly under CACTI_PATH_FORMATS; + // basename() strips any directory traversal or absolute-path component a + // stored report's format_file value might carry. + $format_file = basename($format_file); + + if ($format_file === '' || $format_file === '.' || $format_file === '..') { + return false; + } + $format_file = CACTI_PATH_FORMATS . '/' . $format_file; if (file_exists($format_file) && is_readable($format_file)) { diff --git a/lib/rrd.php b/lib/rrd.php index 1aab661ac..42c15b955 100644 --- a/lib/rrd.php +++ b/lib/rrd.php @@ -1395,9 +1395,9 @@ function rrdtool_function_interface_speed(array $data_local) : string { $ifSpeed = $speeds['ifSpeed'] ?? false; if (!empty($ifHighSpeed)) { - $speed = $ifHighSpeed * 1000000; + $speed = (int) $ifHighSpeed * 1000000; } elseif (!empty($ifSpeed)) { - $speed = $ifSpeed; + $speed = (int) $ifSpeed; } else { $speed = intval(read_config_option('default_interface_speed')); @@ -1631,6 +1631,16 @@ function rrdtool_function_create(int $local_data_id, bool $show_source, mixed $r $data_source['rrd_maximum'] = 'U'; } + // the bounds are about to be concatenated onto an rrdtool command line, so + // never let anything but a number or the literal 'U' reach that sink + foreach (['rrd_minimum', 'rrd_maximum'] as $bound) { + if ($data_source[$bound] !== 'U' && !is_numeric($data_source[$bound])) { + cacti_log("ERROR: Non-numeric $bound '" . $data_source[$bound] . "' for data source '$data_source_name' local_data_id " . $local_data_id, false, 'RRDTOOL'); + + $data_source[$bound] = 'U'; + } + } + $create_ds .= "DS:$data_source_name:" . $data_source_types[$data_source['data_source_type_id']] . ':' . $data_source['rrd_heartbeat'] . ':' . $data_source['rrd_minimum'] . ':' . $data_source['rrd_maximum'] . RRD_NL; } } diff --git a/lib/snmp.php b/lib/snmp.php index 4ae76b769..9fa070942 100644 --- a/lib/snmp.php +++ b/lib/snmp.php @@ -227,7 +227,7 @@ function cacti_snmp_get(string $hostname, mixed $community, string $oid, mixed $ ' -v ' . $version . ' -t ' . $timeout_s . ' -r ' . $retries . - ' ' . cacti_escapeshellarg($hostname) . ':' . $port . + ' ' . cacti_escapeshellarg_cmd($hostname, true, true) . ':' . $port . ' ' . cacti_escapeshellarg($oid); if (isset($_SESSION)) { @@ -315,7 +315,7 @@ function cacti_snmp_get_raw(string $hostname, mixed $community, string $oid, mix ' -v ' . $version . ' -t ' . $timeout_s . ' -r ' . $retries . - ' ' . cacti_escapeshellarg($hostname) . ':' . $port . + ' ' . cacti_escapeshellarg_cmd($hostname, true, true) . ':' . $port . ' ' . cacti_escapeshellarg($oid); if (isset($_SESSION)) { @@ -399,7 +399,7 @@ function cacti_snmp_getnext(string $hostname, mixed $community, mixed $oid, mixe ' -v ' . $version . ' -t ' . $timeout_s . ' -r ' . $retries . - ' ' . cacti_escapeshellarg($hostname) . ':' . $port . + ' ' . cacti_escapeshellarg_cmd($hostname, true, true) . ':' . $port . ' ' . cacti_escapeshellarg($oid); if (isset($_SESSION)) { @@ -877,7 +877,7 @@ function cacti_snmp_walk(string $hostname, mixed $community, string $oid, mixed ' -r ' . $retries . ' -Cr' . $bulk_walk_size . ' ' . $oidCheck . ' ' . - cacti_escapeshellarg($hostname) . ':' . $port . ' ' . + cacti_escapeshellarg_cmd($hostname, true, true) . ':' . $port . ' ' . cacti_escapeshellarg($oid); if (isset($_SESSION)) { @@ -892,7 +892,7 @@ function cacti_snmp_walk(string $hostname, mixed $community, string $oid, mixed ' -t ' . $timeout_s . ' -r ' . $retries . ' ' . $oidCheck . ' ' . - ' ' . cacti_escapeshellarg($hostname) . ':' . $port . + ' ' . cacti_escapeshellarg_cmd($hostname, true, true) . ':' . $port . ' ' . cacti_escapeshellarg($oid); if (isset($_SESSION)) { @@ -1155,16 +1155,11 @@ function format_snmp_string(string $string, bool $snmp_oid_included, int $value_ * @return string Escaped command argument. */ function snmp_escape_string(string $string, string $server_os = CACTI_SERVER_OS) : string { - if (!defined('SNMP_ESCAPE_CHARACTER')) { - define('SNMP_ESCAPE_CHARACTER', '"'); - } - if ($server_os == 'win32') { - if (substr_count($string, SNMP_ESCAPE_CHARACTER)) { - $string = str_replace(SNMP_ESCAPE_CHARACTER, '\\' . SNMP_ESCAPE_CHARACTER, $string); - - return SNMP_ESCAPE_CHARACTER . $string . SNMP_ESCAPE_CHARACTER; - } + /* GHSA-rjvj-r52f-8v5q: cmd.exe ignores the \" escape and toggles + * quoting on every ", so wrapping cannot neutralize & | ^ < > ( ). + * SNMP values never legitimately contain these, so strip them. */ + $string = str_replace(['"', '&', '|', '^', '<', '>', '(', ')'], '', $string); } return cacti_escapeshellarg($string); diff --git a/lib/snmpagent.php b/lib/snmpagent.php index 8cc5ad0e4..61a922382 100644 --- a/lib/snmpagent.php +++ b/lib/snmpagent.php @@ -916,6 +916,10 @@ function snmpagent_notification(string $notification, string $mib, array $varbin $args = []; foreach ($notification_managers as $notification_manager) { + // the receiver hostname is attacker-controlled device data; strip '%' + // here so cmd.exe cannot expand an environment variable from it. + $snmp_trap_receiver = (CACTI_SERVER_OS == 'win32' ? str_replace('%', '', $notification_manager['hostname']) : $notification_manager['hostname']) . ':' . $notification_manager['snmp_port']; + if (!cacti_sizeof($snmp_notification_varbinds)) { foreach ($registered_var_binds as $name => $attributes) { $snmp_notification_varbinds[] = $attributes['oid']; @@ -929,7 +933,7 @@ function snmpagent_notification(string $notification, string $mib, array $varbin $args = array_merge([ '-v', '1', '-c', $notification_manager['snmp_community'], - $notification_manager['hostname'] . ':' . $notification_manager['snmp_port'], + $snmp_trap_receiver, $enterprise_oid, '', '6', $specific_trap_number, '' ], $snmp_notification_varbinds); @@ -941,7 +945,7 @@ function snmpagent_notification(string $notification, string $mib, array $varbin } $args = array_merge($args, [ - $notification_manager['hostname'] . ':' . $notification_manager['snmp_port'], + $snmp_trap_receiver, '', $enterprise_oid ], $snmp_notification_varbinds); } elseif ($notification_manager['snmp_version'] == 3) { @@ -984,7 +988,7 @@ function snmpagent_notification(string $notification, string $mib, array $varbin } $args = array_merge($args, [ - $notification_manager['hostname'] . ':' . $notification_manager['snmp_port'], + $snmp_trap_receiver, '', $enterprise_oid ], $snmp_notification_varbinds); } @@ -995,7 +999,7 @@ function snmpagent_notification(string $notification, string $mib, array $varbin // directly so empty positions survive as quoted '' rather than being // swallowed when exec_background joins the arguments with spaces. $escaped_args = implode(' ', array_map(static function ($arg) { - return $arg === '' ? "''" : cacti_escapeshellarg($arg); + return $arg === '' ? "''" : cacti_escapeshellarg_cmd($arg); }, $args)); exec_background(cacti_escapeshellcmd($path_snmptrap), $escaped_args); diff --git a/package_import.php b/package_import.php index b6de3ed0b..b71160321 100644 --- a/package_import.php +++ b/package_import.php @@ -895,6 +895,16 @@ function package_verify_key() : void { } function package_accept_key() : void { + // Trusting a new signer is a Package Management (realm 29) decision, + // distinct from the Import Templates (realm 17) permission that gates + // this page as a whole. + if (!is_realm_allowed(29)) { + raise_message('permission_denied'); + header('Location: package_import.php'); + + exit; + } + $package_location = gfrv('package_location'); if ($package_location > 0) { @@ -1836,10 +1846,14 @@ function package_import() : void { }); if (checks != '' || $('#package_location').val() == 0) { - $.getJSON('package_import.php?action=accept' + - '&package_location=' + $('#package_location').val() + - '&package_ids=' + checks, function(data) { - }); + /* accept is state-changing (trusts a signer), so it must arrive by + * POST with a CSRF token like the other 'bad_actions' in global.php */ + $.post('package_import.php?action=accept', { + package_location: $('#package_location').val(), + package_ids: checks, + __csrf_magic: csrfMagicToken + }, function(data) { + }, 'json'); } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6eb3faf89..d43ab3507 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1518,6 +1518,12 @@ parameters: count: 4 path: cli/splice_rrd.php + - + message: '#^Function loadTable invoked with 2 parameters, 3 required\.$#' + identifier: arguments.count + count: 1 + path: cli/splice_rrd.php + - message: '#^Parameter \#1 \$output of function preProcessXML expects array, list\\|false given\.$#' identifier: argument.type @@ -4267,11 +4273,17 @@ parameters: path: lib/html_reports.php - - message: '#^Cannot access offset ''user_id'' on array\|bool\.$#' + message: '#^Cannot access offset ''report_id'' on array\|bool\.$#' identifier: offsetAccess.nonOffsetAccessible count: 1 path: lib/html_reports.php + - + message: '#^Cannot access offset ''user_id'' on array\|bool\.$#' + identifier: offsetAccess.nonOffsetAccessible + count: 3 + path: lib/html_reports.php + - message: '#^Function reports_item_validate\(\) should return string but returns string\|false\.$#' identifier: return.type @@ -6954,6 +6966,18 @@ parameters: count: 1 path: poller_realtime.php + - + message: '#^Parameter \#1 \$haystack of function strpos expects string, array\\|string given\.$#' + identifier: argument.type + count: 1 + path: poller_realtime.php + + - + message: '#^Parameter \#1 \$string of function substr expects string, array\\|string given\.$#' + identifier: argument.type + count: 1 + path: poller_realtime.php + - message: '#^Parameter \#3 \$subject of function str_replace expects array\\|string, array\\|string\|null given\.$#' identifier: argument.type diff --git a/poller_realtime.php b/poller_realtime.php index 49bf6efd7..b24679056 100755 --- a/poller_realtime.php +++ b/poller_realtime.php @@ -248,8 +248,14 @@ function process_poller_output_rt(mixed $rrdtool_pipe, string $poller_id, int $i in there (text format) */ $command = str_replace("\\\n", ' ', $command); - // create the rrdfile - shell_exec($command); + // create the rrdfile without going through a shell + $rrdtool = read_config_option('path_rrdtool') . ' '; + + if (strpos($command, $rrdtool) === 0) { + $command = substr($command, strlen($rrdtool)); + } + + rrdtool_execute($command, false, RRDTOOL_OUTPUT_STDOUT, $rrdtool_pipe, 'POLLER'); // change permissions so that the poller can clear @chmod($rt_graph_path, 0644); diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 01b1bf728..3983c4cdc 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,10 +1,5 @@ - - - - - ['regexp' => '/^([-a-zA-Z0-9_\s]+)$/']])]]> diff --git a/tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php b/tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php index 03e061a89..a1e0cba58 100644 --- a/tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php +++ b/tests/Unit/DataCollection/Snmp/SnmpCoverageTest.php @@ -65,6 +65,18 @@ function cacti_escapeshellarg(string $argument) : string { return escapeshellarg($argument); } +function cacti_escapeshellarg_cmd(string $argument, bool $quote = true, bool $strip_env = false) : string { + if (CACTI_SERVER_OS == 'win32') { + $argument = str_replace(['"', '&', '|', '^', '<', '>', '(', ')'], '', $argument); + + if ($strip_env) { + $argument = str_replace('%', '', $argument); + } + } + + return cacti_escapeshellarg($argument, $quote); +} + function debug_log_insert(string $category, string $message) : void { $GLOBALS['snmp_coverage_debug'][] = [$category, $message]; } @@ -296,7 +308,7 @@ test('OID validation, escaping, method selection, options, and v3 auth cover all ->and(cacti_snmp_validate_oid('.'))->toBeFalse() ->and(cacti_snmp_validate_oid('1.bad'))->toBeFalse() ->and(snmp_escape_string('public'))->toBe("'public'") - ->and(snmp_escape_string('a"b', 'win32'))->toBe('"a\\"b"') + ->and(snmp_escape_string('a"b', 'win32'))->toBe("'ab'") ->and(snmp_escape_string('public', 'win32'))->toBe("'public'") ->and(snmp_get_method('get', 1, '', '', SNMP_STRING_OUTPUT_GUESS, false))->toBe(SNMP_METHOD_BINARY) ->and(snmp_get_method('get', 1, '', '', SNMP_STRING_OUTPUT_HEX))->toBe(SNMP_METHOD_BINARY) diff --git a/tests/Unit/Security/Auth/DataSourceEnableDisableAuthzTest.php b/tests/Unit/Security/Auth/DataSourceEnableDisableAuthzTest.php new file mode 100644 index 000000000..1d7365f12 --- /dev/null +++ b/tests/Unit/Security/Auth/DataSourceEnableDisableAuthzTest.php @@ -0,0 +1,84 @@ +not->toBeFalse(); + + $end = strpos($source, "\n}\n", $start); + $body = substr($source, $start, $end - $start); + + expect($body)->toContain('is_device_allowed((int) $host_id)'); + expect($body)->toContain('if (empty($host_id)) {'); +}); + +test('GHSA-2fhv: ds_disable() checks authorization before calling api_data_source_disable()', function () use ($source) { + $start = strpos($source, 'function ds_disable('); + $end = strpos($source, "\n}\n", $start); + $body = substr($source, $start, $end - $start); + + $checkPos = strpos($body, 'data_source_authorized((int) grv(\'id\'))'); + $callPos = strpos($body, 'api_data_source_disable('); + + expect($checkPos)->not->toBeFalse(); + expect($callPos)->not->toBeFalse(); + expect($checkPos)->toBeLessThan($callPos); +}); + +test('GHSA-2fhv: ds_enable() checks authorization before calling api_data_source_enable()', function () use ($source) { + $start = strpos($source, 'function ds_enable('); + $end = strpos($source, "\n}\n", $start); + $body = substr($source, $start, $end - $start); + + $checkPos = strpos($body, 'data_source_authorized((int) grv(\'id\'))'); + $callPos = strpos($body, 'api_data_source_enable('); + + expect($checkPos)->not->toBeFalse(); + expect($callPos)->not->toBeFalse(); + expect($checkPos)->toBeLessThan($callPos); +}); + +test('GHSA-2fhv: the bulk disable/enable actions filter each selected id through data_source_authorized()', function () use ($source) { + expect(substr_count($source, 'if (data_source_authorized((int) $local_data_id)) {'))->toBeGreaterThanOrEqual(2); +}); + +/* + * GHSA-fc82-w6xq-4jg7: the bulk "Change Data Source Profile" action + * (drp_action == '6') iterated $selected_items and updated + * data_template_data/data_template_rrd/poller_item for each local_data_id with + * no object-level authorization check -- the same class of defect as + * GHSA-2fhv, just a different bulk action in the same file. Guarded with the + * same data_source_authorized() helper. + */ +test('GHSA-fc82: the bulk change-data-source-profile action skips unauthorized data sources', function () use ($source) { + $start = strpos($source, "drp_action') == '6') { // change data source profile"); + expect($start)->not->toBeFalse(); + + $end = strpos($source, "\n\t\t\t} elseif", $start); + $body = substr($source, $start, $end - $start); + + $checkPos = strpos($body, 'if (!data_source_authorized((int) $local_data_id)) {'); + $updatePos = strpos($body, "UPDATE data_template_data"); + + expect($checkPos)->not->toBeFalse(); + expect($updatePos)->not->toBeFalse(); + expect($checkPos)->toBeLessThan($updatePos); +}); diff --git a/tests/Unit/Security/Csrf/InstallerCsrfRecoveryTest.php b/tests/Unit/Security/Csrf/InstallerCsrfRecoveryTest.php index 4d1c0aeec..5c59edc92 100644 --- a/tests/Unit/Security/Csrf/InstallerCsrfRecoveryTest.php +++ b/tests/Unit/Security/Csrf/InstallerCsrfRecoveryTest.php @@ -33,7 +33,8 @@ test('installer csrf failures return a scoped json recovery response', function ->and($csrf)->toContain("'csrfMagicToken' => csrf_get_tokens()") ->and($csrf)->toContain("header('Cache-Control: no-store')") ->and($csrf)->toContain("header('X-Content-Type-Options: nosniff')") - ->and($csrf)->toContain("header('Location: ' . sanitize_uri(\$_SERVER['REQUEST_URI']))"); + ->and($csrf)->toContain("validate_redirect_url(\$_SERVER['REQUEST_URI'] ?? '', 'index.php')") + ->and($csrf)->toContain("header('Location: ' . \$redirect_target)"); }); test('successful installer json responses roll the csrf token forward', function () use ($root) { diff --git a/tests/Unit/Security/Misc/DockerApacheHtaccessOverrideTest.php b/tests/Unit/Security/Misc/DockerApacheHtaccessOverrideTest.php new file mode 100644 index 000000000..1bf3040f4 --- /dev/null +++ b/tests/Unit/Security/Misc/DockerApacheHtaccessOverrideTest.php @@ -0,0 +1,36 @@ +toContain('AllowOverride All') + ->and($conf)->not->toContain('AllowOverride FileInfo Options=FollowSymLinks'); +}); + +test('GHSA-fxfg: the Cacti directory block still explicitly grants web access', function () use ($conf) { + $start = strpos($conf, ''); + expect($start)->not->toBeFalse(); + + $end = strpos($conf, '', $start); + $body = substr($conf, $start, $end - $start); + + expect($body)->toContain('Require all granted') + ->and($body)->toContain('AllowOverride All'); +}); diff --git a/tests/Unit/Security/PathTraversal/ReportsFormatFileTraversalTest.php b/tests/Unit/Security/PathTraversal/ReportsFormatFileTraversalTest.php new file mode 100644 index 000000000..c1a19534c --- /dev/null +++ b/tests/Unit/Security/PathTraversal/ReportsFormatFileTraversalTest.php @@ -0,0 +1,57 @@ +not->toBeFalse(); + + $end = strpos($reportsSource, "\n}\n", $start); + $body = substr($reportsSource, $start, $end - $start); + + expect($body)->toContain('$format_file = basename($format_file);'); +}); + +test('GHSA-5x27: reports_load_format_file rejects a traversal-only result before any file IO', function () use ($reportsSource) { + $start = strpos($reportsSource, 'function reports_load_format_file('); + $end = strpos($reportsSource, "\n}\n", $start); + $body = substr($reportsSource, $start, $end - $start); + + $basenamePos = strpos($body, '$format_file = basename($format_file);'); + $rejectPos = strpos($body, "\$format_file === '' || \$format_file === '.' || \$format_file === '..'"); + $existsPos = strpos($body, 'file_exists($format_file)'); + + expect($basenamePos)->not->toBeFalse(); + expect($rejectPos)->not->toBeFalse(); + expect($existsPos)->not->toBeFalse(); + expect($basenamePos)->toBeLessThan($rejectPos); + expect($rejectPos)->toBeLessThan($existsPos); +}); + +test('GHSA-5x27: the save path confines format_file to a bare filename before it reaches the database', function () use ($htmlReportsSource) { + expect($htmlReportsSource)->toContain("\$save['format_file'] = basename((string) (\$post['format_file'] ?? ''));"); +}); + +test('GHSA-5x27: a traversal payload resolves to a harmless bare filename', function () { + $payload = '../../../../../../etc/passwd'; + + expect(basename($payload))->toBe('passwd'); +}); diff --git a/tests/Unit/Security/Shell/CmdMetacharHelperTest.php b/tests/Unit/Security/Shell/CmdMetacharHelperTest.php new file mode 100644 index 000000000..df4ef089f --- /dev/null +++ b/tests/Unit/Security/Shell/CmdMetacharHelperTest.php @@ -0,0 +1,134 @@ + ( )) on Windows before quoting, and the + * Windows shell sinks route their device/request values through it or the + * equivalent snmp_escape_string strip. + */ + +if (!defined('CACTI_SERVER_OS')) { + define('CACTI_SERVER_OS', 'unix'); +} + +if (!defined('SNMP_ESCAPE_CHARACTER')) { + define('SNMP_ESCAPE_CHARACTER', '"'); +} +require_once dirname(__DIR__, 4) . '/lib/functions.php'; + +$functions = file_get_contents(dirname(__DIR__, 4) . '/lib/functions.php'); +$snmp = file_get_contents(dirname(__DIR__, 4) . '/lib/snmp.php'); +$dataquery = file_get_contents(dirname(__DIR__, 4) . '/lib/data_query.php'); +$ping = file_get_contents(dirname(__DIR__, 4) . '/lib/ping.php'); +$snmpagent = file_get_contents(dirname(__DIR__, 4) . '/lib/snmpagent.php'); + +test('cacti_escapeshellarg_cmd exists and strips on win32 by contract', function () use ($functions) { + expect(function_exists('cacti_escapeshellarg_cmd'))->toBeTrue(); + $start = strpos($functions, 'function cacti_escapeshellarg_cmd('); + $body = substr($functions, $start, 500); + expect($body)->toContain("CACTI_SERVER_OS == 'win32'"); + expect($body)->toContain("'^', '<', '>', '('"); +}); + +test('on unix (test default) the helper is a pass-through to cacti_escapeshellarg', function () { + $v = "eth0' && id"; + expect(cacti_escapeshellarg_cmd($v))->toBe(cacti_escapeshellarg($v)); +}); + +test('snmp_escape_string strips cmd.exe operators on win32', function () use ($snmp) { + $start = strpos($snmp, 'function snmp_escape_string('); + $body = substr($snmp, $start, 700); + expect($body)->toContain("'^', '<', '>', '('"); +}); + +test('the snmp binary hostname uses the cmd helper', function () use ($snmp) { + expect($snmp)->toContain('cacti_escapeshellarg_cmd($hostname, true, true)'); +}); + +test('get_script_query_path strips device values on win32', function () use ($dataquery) { + $start = strpos($dataquery, 'function get_script_query_path('); + $body = substr($dataquery, $start, 1200); + expect($body)->toContain("CACTI_SERVER_OS == 'win32'"); + expect($body)->toContain("'^', '<', '>', '('"); +}); + +test('ping hostname and snmp trap args use the cmd helper', function () use ($ping, $snmpagent) { + expect($ping)->toContain('cacti_escapeshellarg_cmd($this->host[' . "'hostname'" . '], true, true)'); + expect($snmpagent)->toContain('cacti_escapeshellarg_cmd($arg)'); +}); + +test('the strip_env flag removes %VAR% for hostname-class values only', function () { + // default: percent is preserved (credentials may contain it) + expect(cacti_escapeshellarg_cmd('p%ss'))->toBe(cacti_escapeshellarg('p%ss')); + // opt-in: a hostname never contains a percent, so it is stripped on win32. + // on unix (test default) the flag is a no-op, so assert the source wiring. + $src = file_get_contents(dirname(__DIR__, 4) . '/lib/functions.php'); + $start = strpos($src, 'function cacti_escapeshellarg_cmd('); + $body = substr($src, $start, 700); + expect($body)->toContain('$strip_env'); + expect($body)->toContain("str_replace('%', ''"); +}); + +test('the hostname sinks opt into the percent strip', function () { + $snmp = file_get_contents(dirname(__DIR__, 4) . '/lib/snmp.php'); + $ping = file_get_contents(dirname(__DIR__, 4) . '/lib/ping.php'); + expect($snmp)->toContain('cacti_escapeshellarg_cmd($hostname, true, true)'); + expect($ping)->toContain('cacti_escapeshellarg_cmd($this->host[' . "'hostname'" . '], true, true)'); +}); + +/* + * CACTI_SERVER_OS is a constant, so the win32 branch above can only be + * inferred from source text in this process. Run the helper in a fresh + * process with CACTI_SERVER_OS actually set to 'win32' so the stripping + * really executes instead of being assumed from the assertions above. + */ +test('cacti_escapeshellarg_cmd actually strips metachars and percent on win32', function () { + $result = cmd_metachar_run_win32_probe(); + + expect($result['metachars'])->toBe('"abcdefghi"') + ->and($result['percent-default'])->toBe('"host%PATH%name"') + ->and($result['percent-stripped'])->toBe('"hostPATHname"') + ->and($result['credential-kept'])->toBe('"p%ss"'); +}); + +/** + * Run tests/fixtures/CmdMetacharHelperWin32Probe.php and parse its + * "label\tresult" stdout lines. + * + * @return array Result keyed by test case label. + */ +function cmd_metachar_run_win32_probe() : array { + $root = dirname(__DIR__, 4); + + $process = proc_open( + [PHP_BINARY, $root . '/tests/fixtures/CmdMetacharHelperWin32Probe.php'], + [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], + $pipes, + $root + ); + + if (!is_resource($process)) { + throw new RuntimeException('Unable to start the win32 metachar probe'); + } + + $stdout = stream_get_contents($pipes[1]); + fclose($pipes[1]); + fclose($pipes[2]); + proc_close($process); + + $results = []; + + foreach (explode("\n", trim($stdout)) as $line) { + [$label, $value] = explode("\t", $line, 2); + $results[$label] = $value; + } + + return $results; +} diff --git a/tests/Unit/Security/SqlInjection/SafeSortTest.php b/tests/Unit/Security/SqlInjection/SafeSortTest.php index ade21a8e0..bef31bac1 100644 --- a/tests/Unit/Security/SqlInjection/SafeSortTest.php +++ b/tests/Unit/Security/SqlInjection/SafeSortTest.php @@ -34,7 +34,17 @@ // --- Stub: sanitize_sql_column (lib/functions.php) --- function stub_sanitize_sql_column(string $column): string { - return preg_replace('/[^a-zA-Z0-9_().]/', '', $column) ?? ''; + $result = preg_replace('/[^a-zA-Z0-9_().]/', '', $column) ?? ''; + + if ($result === '') { + return ''; + } + + if (str_contains($result, '(') && preg_match('/^(?:INET_ATON|NATURAL_SORT_KEY)\([a-zA-Z_][a-zA-Z0-9_.]*\)$/i', $result) !== 1) { + return ''; + } + + return $result; } // --- Stub: direction enforcement used in html_reports.php, api_automation.php, @@ -58,6 +68,31 @@ test('sanitize_sql_column: function call with parens passes', function () { expect(stub_sanitize_sql_column('INET_ATON(ip)'))->toBe('INET_ATON(ip)'); }); +test('sanitize_sql_column: NATURAL_SORT_KEY wrapper passes', function () { + expect(stub_sanitize_sql_column('NATURAL_SORT_KEY(hostname)'))->toBe('NATURAL_SORT_KEY(hostname)'); +}); + +test('sanitize_sql_column: subquery payload bypassing GHSA-q9xg-p762-9jm3 is rejected (GHSA-8j73-cvf6-jrvx)', function () { + $payload = '(SELECT(0)FROM(SELECT(SLEEP(5)))a)'; + $output = stub_sanitize_sql_column($payload); + + expect($output)->not->toBe($payload); + expect(strtoupper($output))->not->toContain('SELECT'); + expect(strtoupper($output))->not->toContain('SLEEP'); +}); + +test('sanitize_sql_column: REGEXP exfiltration subquery payload is rejected (GHSA-8j73-cvf6-jrvx)', function () { + $payload = '(SELECT(0)FROM(SELECT(SLEEP(5))FROM(user_auth)WHERE(BINARY(username)REGEXP(0x5e61646d696e24)))a)'; + $output = stub_sanitize_sql_column($payload); + + expect(strtoupper($output))->not->toContain('SELECT'); + expect(strtoupper($output))->not->toContain('FROM'); +}); + +test('sanitize_sql_column: a function name that is not on the wrapper allowlist is rejected', function () { + expect(stub_sanitize_sql_column('SLEEP(5)'))->not->toBe('SLEEP(5)'); +}); + test('sanitize_sql_column: column with underscore and digits passes', function () { expect(stub_sanitize_sql_column('last_started_2'))->toBe('last_started_2'); }); diff --git a/tests/fixtures/CmdMetacharHelperWin32Probe.php b/tests/fixtures/CmdMetacharHelperWin32Probe.php new file mode 100644 index 000000000..785553e38 --- /dev/null +++ b/tests/fixtures/CmdMetacharHelperWin32Probe.php @@ -0,0 +1,43 @@ + cacti_escapeshellarg_cmd('a"b&c|d^eg(h)i'), + 'percent-default' => cacti_escapeshellarg_cmd('host%PATH%name'), + 'percent-stripped' => cacti_escapeshellarg_cmd('host%PATH%name', true, true), + 'credential-kept' => cacti_escapeshellarg_cmd('p%ss'), +]; + +foreach ($cases as $label => $result) { + echo $label . "\t" . $result . "\n"; +} diff --git a/tests/fixtures/CsrfCheckContractProbe.php b/tests/fixtures/CsrfCheckContractProbe.php index fc4953da8..d832d2b9b 100644 --- a/tests/fixtures/CsrfCheckContractProbe.php +++ b/tests/fixtures/CsrfCheckContractProbe.php @@ -84,6 +84,14 @@ if (!function_exists('sanitize_uri')) { } } +// lib/html_utility.php is not loaded here; csrf_error_callback()'s redirect +// branch only needs a default to fall back to, not the real validation. +if (!function_exists('validate_redirect_url')) { + function validate_redirect_url($url = '', $default = 'index.php') { + return $default; + } +} + $mode = ($argv[1] ?? ''); $_SERVER['REQUEST_URI'] = '/test'; diff --git a/tests/security/baselines/architectural_hotspots.baseline.tsv b/tests/security/baselines/architectural_hotspots.baseline.tsv index 6947c9d37..1adb2018d 100644 --- a/tests/security/baselines/architectural_hotspots.baseline.tsv +++ b/tests/security/baselines/architectural_hotspots.baseline.tsv @@ -41,23 +41,24 @@ cmd_exec ./lib/functions.php $output = shell_exec('powershell -Command "Get-Wm cmd_exec ./lib/functions.php $processExists = trim((string) shell_exec($checkProcessCmd)); cmd_exec ./lib/functions.php exec($command_line, $out, $return_code); cmd_exec ./lib/functions.php return exec('whoami') ?: ''; +cmd_exec ./lib/functions.php * On Windows, exec()/shell_exec()/popen() route through cmd.exe, which ignores cmd_exec ./lib/installer.php $output = shell_exec( cmd_exec ./lib/installer.php $output = shell_exec(cacti_escapeshellcmd(str_replace('\\', '/', $path)) . ' --version 2>&1'); cmd_exec ./lib/installer.php $check = shell_exec("$composer install --dry-run --no-interaction --working-dir=$base 2>&1"); cmd_exec ./lib/installer.php $output = shell_exec("$composer install --no-interaction --no-progress --working-dir=$base 2>&1"); cmd_exec ./lib/installer.php $process = proc_open( -cmd_exec ./lib/ping.php $result = shell_exec('ping -6 -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg($this->host['hostname']) . ' 2>&1'); -cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping6 -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('chcp 437 && ping -w ' . $this->timeout . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -i ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -m ' . ceil($this->timeout / 1000) . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -w ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec(cacti_escapeshellarg($fping) . ' -q -t ' . $this->timeout . ' -c 1 -r ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname']) . ' 2>&1'); +cmd_exec ./lib/ping.php $result = shell_exec('ping -6 -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true) . ' 2>&1'); +cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping6 -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('chcp 437 && ping -w ' . $this->timeout . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -i ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -m ' . ceil($this->timeout / 1000) . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -w ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec(cacti_escapeshellarg($fping) . ' -q -t ' . $this->timeout . ' -c 1 -r ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true) . ' 2>&1'); cmd_exec ./lib/poller.php $output = system($php_path . ' -l ' . $tmpfile, $exit); cmd_exec ./lib/poller.php pclose(popen('start "Cactiplus" /I ' . $filename . ' ' . $args . ' ' . $redirect_args, 'r')); cmd_exec ./lib/poller.php pclose(popen('start "Cactiplus" /I ' . $filename . ' ' . $args, 'r')); @@ -92,7 +93,44 @@ cmd_exec ./lib/utility.php exec('wmic os get TotalVisibleMemorySize', $memInfo cmd_exec ./lib/utility.php $json = shell_exec($php . ' -q ' . $php_file); cmd_exec ./lib/utility.php $json = shell_exec($php . ' -q ' . $php_file); cmd_exec ./lib/utility.php $json = shell_exec($php . ' -q ' . $php_file); -cmd_exec ./poller_realtime.php shell_exec($command); +cmd_exec ./plugins/analytics/lib/functions.php $slotsTotal = trim((string) shell_exec($rrdtool . " graph /dev/null -s $min_time -e $max_time DEF:jobs='$rrd_file':'max_jobs':MAX PRINT:jobs:MAX:'%.0lf' | tail -1")); +cmd_exec ./plugins/analytics/lib/functions.php $value = trim((string) shell_exec($rrdtool . " graph /dev/null -s $rrd_min -e $rrd_max DEF:data=$rrd_file:$data_source:$cf PRINT:data:MAX:'%.0lf' | tail -1")); +cmd_exec ./plugins/analytics/lib/functions.php $value = trim((string) shell_exec($rrdtool . " graph /dev/null -s $rrd_min -e $rrd_max DEF:data=$rrd_file:$data_source:$cf PRINT:data:MAX:'%.0lf' | tail -1")); +cmd_exec ./plugins/analytics/lib/functions.php $value = trim((string) shell_exec($rrdtool . " graph /dev/null -s $rrd_min -e $rrd_max DEF:data=$rrd_file:$data_source:AVERAGE PRINT:data:MAX:'%.0lf' | tail -1")); +cmd_exec ./plugins/analytics/lib/functions.php $bjgroup_slots = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bjgroup -N 2>&1")); +cmd_exec ./plugins/analytics/lib/functions.php $data = (string) shell_exec($rrdtool . " xport -t -s $rrd_min -e $rrd_max --step $desired_step DEF:data=$rrd_file:$data_source:$cf XPORT:data:$data_source"); +cmd_exec ./plugins/analytics/lib/functions.php $lsinfo = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/lsinfo -w 2>&1")); +cmd_exec ./plugins/analytics/lib/functions.php $output = (string) shell_exec("pstree -p $pid"); +cmd_exec ./plugins/analytics/lib/functions.php $reasons = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bjobs -psum -p1 -uall -sla $sla 2>&1")); +cmd_exec ./plugins/analytics/lib/functions.php $bjgroup_jobs = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bjgroup -s 2>&1")); +cmd_exec ./plugins/analytics/lib/functions.php $bmgroup = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bmgroup -w 2>&1")); +cmd_exec ./plugins/analytics/lib/functions.php $bresources = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bresources -g -l $group 2>&1 | grep 'HOSTS:'")); +cmd_exec ./plugins/analytics/lib/functions.php $bresources = trim((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bresources -g 2>&1")); +cmd_exec ./plugins/analytics/lib/functions.php $jobs = file((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bjobs -uall -UF -plan 2>/dev/null")); +cmd_exec ./plugins/analytics/lib/functions.php $jobs = json_decode((string) shell_exec($config['base_path'] . "/plugins/analytics/bin/bjobs -uall -o 'jobid jobindex user project app stat queue sla:60 first_host slots min_req_proc max_req_proc cpu_used max_mem mem run_time submit_time start_time finish_time effective_resreq combined_resreq' -json 2>/dev/null"), true); +cmd_exec ./plugins/analytics/lib/functions.php $reasons = trim((string) shell_exec($config['base_path'] . '/plugins/analytics/bin/bjobs -psum -p1 -uall 2>&1')); +cmd_exec ./plugins/analytics/lib/functions.php $suspend = (string) shell_exec($config['base_path'] . '/plugins/analytics/bin/bjobs -uall -s 2>&1'); +cmd_exec ./plugins/analytics/lib/functions.php $values = (string) shell_exec(read_config_option('path_rrdtool') . " fetch $rrd_path $cf $resolution -s $start_time -e $end_time"); +cmd_exec ./plugins/analytics/lib/functions.php $values = (string) shell_exec(read_config_option('path_rrdtool') . " fetch $rrd_path $cf $resolution -s $start_time -e $end_time"); +cmd_exec ./plugins/backup/poller_backup.php shell_exec("chown $runas:$runas $tmpfile"); +cmd_exec ./plugins/backup/poller_backup.php shell_exec("chown $runas:$runas $tmpsqlgzfile"); +cmd_exec ./plugins/backup/poller_backup.php shell_exec("chown $dbrunas:$dbrunas $tmpsqlfile"); +cmd_exec ./plugins/backup/poller_backup.php exec($command, $output, $return); +cmd_exec ./plugins/cjobs/setup.php exec($action, $output, $return); +cmd_exec ./plugins/gexport/functions.php exec('rsync -q ' . $export['export_args'] . ' ' . $prune . $keyopt . ' ' . $stExportDir . '/. ' . ($user != '' ? "$user@":'') . $host . ':' . $export['export_directory'] . ' 2>&1', $output, $retvar); +cmd_exec ./plugins/gexport/functions.php exec('scp ' . $export['export_args'] . ' ' . $keyopt . ($port != '' ? ' -P ' . "$port ":"") . $stExportDir . '/. ' . ($user != '' ? "$user@":'') . $host . ':' . $export['export_directory'] . ' 2>&1', $output, $retvar); +cmd_exec ./plugins/gexport/functions.php system($stExecute, $iExecuteReturns); +cmd_exec ./plugins/hmib/poller_graphs.php $results = shell_exec($command); +cmd_exec ./plugins/hmib/poller_graphs.php $result = exec("$php_bin -q $base/cli/add_device.php --description='Summary Device' --ip=summary --template=$host_template --version=0 --avail=none", $return_code); +cmd_exec ./plugins/hmib/poller_graphs.php $result = exec("$php_bin -q $base/cli/poller_reindex_hosts.php -id=$host_id -qid=All", $return_code); +cmd_exec ./plugins/hmib/snmp.php exec(cacti_escapeshellcmd(read_config_option('path_snmpget')) . ' -O fntevU ' . $snmp_auth . " -v $version -t $timeout -r $retries " . cacti_escapeshellarg($hostname) . ":$port " . cacti_escapeshellarg($oid), $snmp_value); +cmd_exec ./plugins/hmib/snmp.php exec(cacti_escapeshellcmd(read_config_option('path_snmpgetnext')) . " -O fntevU $snmp_auth -v $version -t $timeout -r $retries " . cacti_escapeshellarg($hostname) . ":$port " . cacti_escapeshellarg($oid), $snmp_value); +cmd_exec ./plugins/lsflog/lsflog_cronjob.php $size = shell_exec("du -shBG $logdir | awk '{print $1}' | sed 's/[^1-9]//g' | tr -d '\n'"); +cmd_exec ./plugins/lsflog/lsflog_cronjob.php shell_exec('find ' . $logdir . '/* -mtime ' . $duration . ' -exec /bin/rm -f {} \;'); +cmd_exec ./plugins/lsflog/lsflog_cronjob.php $files = shell_exec('find ' . $logdir . ' -name \*.benchmark -cmin -160'); +cmd_exec ./plugins/lsflog/lsflog_cronjob.php $size = shell_exec("du -shBG $logdir | awk '{print $1}' | sed 's/[^1-9]//g' | tr -d '\n'"); +cmd_exec ./plugins/lsflog/lsflog_cronjob.php shell_exec('find ' . $logdir . '/* -mtime ' . $duration . ' -exec /bin/rm -f {} \;'); +cmd_exec ./plugins/mikrotik/poller_graphs.php $lline = exec($command, $output, $return); cmd_exec ./poller_realtime.php shell_exec("$command_string $extra_args"); cmd_exec ./poller_spikekill.php $response = exec(cacti_escapeshellcmd((string) read_config_option('path_php_binary')) . ' -q ' . cmd_exec ./remote_agent.php $cactiphp = proc_open(read_config_option('path_php_binary') . ' -q ' . CACTI_PATH_BASE . '/script_server.php realtime ' . cacti_escapeshellarg($poller_id), $cactides, $pipes); @@ -125,9 +163,110 @@ dynamic_include ./lib/rrd.php include($rrdtheme); dynamic_include ./lib/rrd.php include($themefile); dynamic_include ./link.php require_once($file); dynamic_include ./plugins.php require_once($setup_file); +dynamic_include ./plugins/analytics/analytics_panels.php include_once($file); +dynamic_include ./plugins/analytics/analytics_panels.php include_once($panel['pfile']); +dynamic_include ./plugins/analytics/lib/archive.php include_once($config['base_path'] . '/plugins/grid/lib/grid_partitioning.php'); +dynamic_include ./plugins/analytics/lib/archive.php include_once($config['base_path'] . '/plugins/grid/lib/grid_partitioning.php'); +dynamic_include ./plugins/analytics/lib/archive.php include_once($config['base_path'] . '/plugins/grid/lib/grid_partitioning.php'); +dynamic_include ./plugins/analytics/lib/archive.php include_once($config['base_path'] . '/plugins/grid/lib/grid_partitioning.php'); +dynamic_include ./plugins/analytics/lib/archive.php include_once($config['base_path'] . '/plugins/grid/lib/grid_partitioning.php'); +dynamic_include ./plugins/analytics/lib/functions.php include_once($file); +dynamic_include ./plugins/analytics/poller_etl.php include($config['include_path'] . '/config.php'); +dynamic_include ./plugins/analytics/setup.php include_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/analytics/setup.php include_once($config['base_path'] . '/lib/reports.php'); +dynamic_include ./plugins/analytics/setup.php include_once($config['base_path'] . '/lib/reports.php'); +dynamic_include ./plugins/analytics/setup.php include_once($config['base_path'] . '/lib/rtm_functions.php'); +dynamic_include ./plugins/analytics/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/analytics/setup.php include_once($config['library_path'] . '/functions.php'); +dynamic_include ./plugins/analytics/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/analytics/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/audit/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/audit/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/audit/setup.php include_once($config['library_path'] . '/functions.php'); +dynamic_include ./plugins/audit/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/audit/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/backup/export_device_templates.php include_once($config['base_path'] . '/plugins/backup/setup.php'); +dynamic_include ./plugins/backup/export_device_templates.php include_once($config['library_path'] . '/export.php'); +dynamic_include ./plugins/backup/poller_backup.php include_once($config['base_path'] . '/plugins/backup/setup.php'); +dynamic_include ./plugins/backup/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/backup/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/cjobs/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/cjobs/setup.php include_once($config['library_path'] . '/functions.php'); +dynamic_include ./plugins/cjobs/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/cjobs/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/evidence/include/functions.php include_once($config['library_path'] . '/poller.php'); +dynamic_include ./plugins/evidence/include/functions.php include_once($config['base_path'] . '/plugins/evidence/include/arrays.php'); +dynamic_include ./plugins/evidence/include/functions.php include_once($config['base_path'] . '/plugins/evidence/include/arrays.php'); +dynamic_include ./plugins/evidence/poller_evidence.php include_once($config['base_path'] . '/plugins/evidence/setup.php'); +dynamic_include ./plugins/evidence/poller_evidence.php include_once($config['base_path'] . '/plugins/evidence/include/functions.php'); +dynamic_include ./plugins/evidence/poller_evidence.php include_once($config['library_path'] . '/snmp.php'); +dynamic_include ./plugins/evidence/setup.php include_once($config['base_path'] . '/plugins/evidence/include/database.php'); +dynamic_include ./plugins/evidence/setup.php include_once($config['base_path'] . '/plugins/evidence/include/database.php'); +dynamic_include ./plugins/evidence/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/evidence/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/exlist/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/exlist/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/gexport/poller_export.php include_once($config['base_path'] . '/plugins/gexport/setup.php'); +dynamic_include ./plugins/gexport/poller_export.php include_once($config['base_path'] . '/lib/data_query.php'); +dynamic_include ./plugins/gexport/poller_export.php include_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/gexport/poller_export.php include_once($config['base_path'] . '/lib/rrd.php'); +dynamic_include ./plugins/gexport/poller_export.php include_once($config['base_path'] . '/plugins/gexport/functions.php'); +dynamic_include ./plugins/gexport/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/gexport/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/gexport/setup.php include_once($config['library_path'] . '/functions.php'); +dynamic_include ./plugins/gexport/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/gexport/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/hmib/associate_os_type.php include_once($config['base_path'] . '/plugins/hmib/setup.php'); +dynamic_include ./plugins/hmib/poller_graphs.php include_once($config['base_path'] . '/plugins/hmib/setup.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/api_automation.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/api_automation_tools.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/api_data_source.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/api_device.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/api_graph.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/data_query.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/snmp.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/sort.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/template.php'); +dynamic_include ./plugins/hmib/poller_graphs.php require_once($config['base_path'] . '/lib/utility.php'); +dynamic_include ./plugins/hmib/poller_hmib.php include_once($config['base_path'] . '/plugins/hmib/setup.php'); +dynamic_include ./plugins/hmib/setup.php include_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/hmib/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/hmib/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/hmib/setup.php include_once($config['library_path'] . '/functions.php'); +dynamic_include ./plugins/hmib/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/hmib/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/lsflog/lsflog_cronjob.php include_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/lsflog/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/lsflog/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/mikrotik/poller_graphs.php include_once($config['base_path'] . '/plugins/mikrotik/setup.php'); +dynamic_include ./plugins/mikrotik/poller_mikrotik.php include_once($config['base_path'] . '/plugins/mikrotik/setup.php'); +dynamic_include ./plugins/mikrotik/setup.php include_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/mikrotik/setup.php include_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/mikrotik/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/mikrotik/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/mikrotik/tests/stubs/lib/database.php * include_once($config['library_path'] . '/database.php') before creating +dynamic_include ./plugins/schema/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/schema/tests/bootstrap-unit.php require_once $autoload; +dynamic_include ./plugins/tags/include/database.php include_once($config['base_path'] . '/plugins/tags/include/functions.php'); +dynamic_include ./plugins/tags/include/functions.php require($config['base_path'] . '/plugins/tags/include/arrays.php'); +dynamic_include ./plugins/tags/poller_tags.php include_once($config['base_path'] . '/plugins/tags/setup.php'); +dynamic_include ./plugins/tags/poller_tags.php require_once($config['base_path'] . '/lib/poller.php'); +dynamic_include ./plugins/tags/poller_tags.php require_once($config['base_path'] . '/plugins/tags/include/functions.php'); +dynamic_include ./plugins/tags/setup.php require_once($config['base_path'] . '/plugins/tags/include/functions.php'); +dynamic_include ./plugins/tags/setup.php include_once($config['base_path'] . '/plugins/tags/include/arrays.php'); +dynamic_include ./plugins/tags/setup.php include_once($config['base_path'] . '/plugins/tags/include/database.php'); +dynamic_include ./plugins/tags/setup.php include_once($config['base_path'] . '/plugins/tags/include/database.php'); +dynamic_include ./plugins/tags/setup.php require_once($config['library_path'] . '/database.php'); +dynamic_include ./plugins/tags/tags.php include_once($config['base_path'] . '/lib/timespan_settings.php'); +dynamic_include ./plugins/tags/tags.php require($config['base_path'] . '/plugins/tags/include/arrays.php'); +dynamic_include ./plugins/tags/tags.php require_once($config['base_path'] . '/plugins/tags/include/functions.php'); +dynamic_include ./plugins/tags/tests/bootstrap-unit.php require_once $path; +dynamic_include ./plugins/tags/tests/bootstrap-unit.php require_once $autoload; dynamic_include ./reports.php require_once($reportit_api); dynamic_include ./script_server.php require_once($include_file); dynamic_include ./scripts/ss_mikrotik_snmpget.php include_once($config['base_path'] . '/lib/snmp.php'); dynamic_include ./scripts/ss_netsnmp_lmsensors.php include($config['base_path'] . '/lib/snmp.php'); dynamic_include ./snmpagent_persist.php require($path_mibcache); host_header ./lib/auth.php $result['link'] = \Sonata\GoogleAuthenticator\GoogleQrUrl::generate($current_user['username'] . '@' . $_SERVER['HTTP_HOST'], $current_user['tfa_secret'], 'Cacti'); +sort_order_input ./plugins/lsflog/grid_lsflog.php ORDER BY " . get_request_var('sort_column') . ' ' . get_request_var('sort_direction'); diff --git a/tests/security/baselines/sink_inventory.baseline.tsv b/tests/security/baselines/sink_inventory.baseline.tsv index 404267d9b..5d492bcf2 100644 --- a/tests/security/baselines/sink_inventory.baseline.tsv +++ b/tests/security/baselines/sink_inventory.baseline.tsv @@ -41,23 +41,24 @@ cmd_exec ./lib/functions.php $output = shell_exec('powershell -Command "Get-Wm cmd_exec ./lib/functions.php $processExists = trim((string) shell_exec($checkProcessCmd)); cmd_exec ./lib/functions.php exec($command_line, $out, $return_code); cmd_exec ./lib/functions.php return exec('whoami') ?: ''; +cmd_exec ./lib/functions.php * On Windows, exec()/shell_exec()/popen() route through cmd.exe, which ignores cmd_exec ./lib/installer.php $output = shell_exec( cmd_exec ./lib/installer.php $output = shell_exec(cacti_escapeshellcmd(str_replace('\\', '/', $path)) . ' --version 2>&1'); cmd_exec ./lib/installer.php $check = shell_exec("$composer install --dry-run --no-interaction --working-dir=$base 2>&1"); cmd_exec ./lib/installer.php $output = shell_exec("$composer install --no-interaction --no-progress --working-dir=$base 2>&1"); cmd_exec ./lib/installer.php $process = proc_open( -cmd_exec ./lib/ping.php $result = shell_exec('ping -6 -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg($this->host['hostname']) . ' 2>&1'); -cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping6 -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('chcp 437 && ping -w ' . $this->timeout . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -i ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -m ' . ceil($this->timeout / 1000) . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec('ping -w ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname'])); -cmd_exec ./lib/ping.php $result = shell_exec(cacti_escapeshellarg($fping) . ' -q -t ' . $this->timeout . ' -c 1 -r ' . $this->retries . ' ' . cacti_escapeshellarg($this->host['hostname']) . ' 2>&1'); +cmd_exec ./lib/ping.php $result = shell_exec('ping -6 -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -W ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' -p ' . $pattern . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true) . ' 2>&1'); +cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping6 -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('chcp 437 && ping -w ' . $this->timeout . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -i ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -m ' . ceil($this->timeout / 1000) . ' -n ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -t ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec('ping -w ' . ceil($this->timeout / 1000) . ' -c ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true)); +cmd_exec ./lib/ping.php $result = shell_exec(cacti_escapeshellarg($fping) . ' -q -t ' . $this->timeout . ' -c 1 -r ' . $this->retries . ' ' . cacti_escapeshellarg_cmd($this->host['hostname'], true, true) . ' 2>&1'); cmd_exec ./lib/poller.php $output = system($php_path . ' -l ' . $tmpfile, $exit); cmd_exec ./lib/poller.php pclose(popen('start "Cactiplus" /I ' . $filename . ' ' . $args . ' ' . $redirect_args, 'r')); cmd_exec ./lib/poller.php pclose(popen('start "Cactiplus" /I ' . $filename . ' ' . $args, 'r')); @@ -92,7 +93,6 @@ cmd_exec ./lib/utility.php exec('wmic os get TotalVisibleMemorySize', $memInfo cmd_exec ./lib/utility.php $json = shell_exec($php . ' -q ' . $php_file); cmd_exec ./lib/utility.php $json = shell_exec($php . ' -q ' . $php_file); cmd_exec ./lib/utility.php $json = shell_exec($php . ' -q ' . $php_file); -cmd_exec ./poller_realtime.php shell_exec($command); cmd_exec ./poller_realtime.php shell_exec("$command_string $extra_args"); cmd_exec ./poller_spikekill.php $response = exec(cacti_escapeshellcmd((string) read_config_option('path_php_binary')) . ' -q ' . cmd_exec ./remote_agent.php $cactiphp = proc_open(read_config_option('path_php_binary') . ' -q ' . CACTI_PATH_BASE . '/script_server.php realtime ' . cacti_escapeshellarg($poller_id), $cactides, $pipes); @@ -387,6 +387,8 @@ header_redirect ./data_sources.php header('Location: data_sources.php?ac header_redirect ./data_sources.php header('Location: data_sources.php'); header_redirect ./data_sources.php header('Location: data_sources.php'); header_redirect ./data_sources.php header('Location: data_sources.php'); +header_redirect ./data_sources.php header('Location: data_sources.php'); +header_redirect ./data_sources.php header('Location: data_sources.php'); header_redirect ./data_sources.php header('Location: data_sources.php?action=ds_edit&host_id=' . grv('host_id') . '&new=1'); header_redirect ./data_sources.php header('Location: data_sources.php?action=ds_edit&id=' . (empty($local_data_id) ? gfrv('local_data_id') : $local_data_id) . '&host_id=' . grv('host_id') . '&view_rrd=' . (isrv('current_rrd') ? gnrv('current_rrd') : '0')); header_redirect ./data_sources.php header('Location: data_sources.php?action=ds_edit&id=' . grv('id') . "&view_rrd=$data_template_rrd_id"); @@ -445,7 +447,7 @@ header_redirect ./include/auth.php header('Location: ' . CACTI_PATH_URL . 'a header_redirect ./include/auth.php header('Location: ' . CACTI_PATH_URL . 'auth_changepassword.php?ref=' . rawurlencode(validate_redirect_url($_SERVER['HTTP_REFERER'] ?? '', 'index.php'))); header_redirect ./include/auth.php header('Location: ' . CACTI_PATH_URL . 'install/'); header_redirect ./include/content/index.php header('Location:../index.php'); -header_redirect ./include/csrf.php header('Location: ' . sanitize_uri($_SERVER['REQUEST_URI'])); +header_redirect ./include/csrf.php header('Location: ' . $redirect_target); header_redirect ./include/fa/css/index.php header('Location:../index.php'); header_redirect ./include/fa/index.php header('Location:../index.php'); header_redirect ./include/fa/js/index.php header('Location:../index.php'); @@ -514,6 +516,12 @@ header_redirect ./lib/html_graph.php header('Location: permission_denied.php') header_redirect ./lib/html_graph.php header('Location: permission_denied.php'); header_redirect ./lib/html_graph.php header('Location: permission_denied.php'); header_redirect ./lib/html_graph.php header('Location: graph_view.php?action=list'); +header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); +header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); +header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); +header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); +header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); +header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page() . '?action=item_edit&id=' . gnrv('report_id') . '&item_id=' . (empty($item_id) ? gnrv('id') : $item_id)); header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); header_redirect ./lib/html_reports.php header('Location: ' . get_reports_page()); @@ -556,6 +564,7 @@ header_redirect ./package_import.php header('Location: package_import.php'); header_redirect ./package_import.php header('Location: package_import.php'); header_redirect ./package_import.php header('Location: package_import.php?package_location=0'); header_redirect ./package_import.php header('Location: package_import.php'); +header_redirect ./package_import.php header('Location: package_import.php'); header_redirect ./package_import.php header('Location: package_import.php?package_location=' . $package_location); header_redirect ./package_keys.php header('Location: package_keys.php?header=false'); header_redirect ./package_keys.php header('Location: package_keys.php?header=false'); @@ -607,6 +616,11 @@ header_redirect ./templates_export.php header('Location: templates_export.ph header_redirect ./templates_import.php header('Location: templates_import.php'); header_redirect ./templates_import.php header('Location: templates_import.php'); header_redirect ./tests/index.php header('Location:../index.php'); +header_redirect ./tree.php header('Location: tree.php'); +header_redirect ./tree.php header('Location: tree.php'); +header_redirect ./tree.php header('Location: tree.php'); +header_redirect ./tree.php header('Location: tree.php'); +header_redirect ./tree.php header('Location: tree.php'); header_redirect ./tree.php header('Location: tree.php'); header_redirect ./tree.php header('Location: tree.php'); header_redirect ./tree.php header('Location: tree.php'); diff --git a/tree.php b/tree.php index c71b7aed0..57cf7645a 100644 --- a/tree.php +++ b/tree.php @@ -144,22 +144,57 @@ switch (grv('action')) { break; case 'copy_node': + if (!is_tree_allowed((int) grv('tree_id'))) { + raise_message('tree_idor', __('You do not have permission to modify this tree.'), MESSAGE_LEVEL_ERROR); + header('Location: tree.php'); + + exit; + } + api_tree_copy_node(grv('tree_id'), grv('id'), grv('parent'), grv('position')); break; case 'create_node': + if (!is_tree_allowed((int) grv('tree_id'))) { + raise_message('tree_idor', __('You do not have permission to modify this tree.'), MESSAGE_LEVEL_ERROR); + header('Location: tree.php'); + + exit; + } + api_tree_create_node(grv('tree_id'), grv('id'), grv('position'), gnrv('text')); break; case 'delete_node': + if (!is_tree_allowed((int) grv('tree_id'))) { + raise_message('tree_idor', __('You do not have permission to modify this tree.'), MESSAGE_LEVEL_ERROR); + header('Location: tree.php'); + + exit; + } + api_tree_delete_node(grv('tree_id'), grv('id')); break; case 'move_node': + if (!is_tree_allowed((int) grv('tree_id'))) { + raise_message('tree_idor', __('You do not have permission to modify this tree.'), MESSAGE_LEVEL_ERROR); + header('Location: tree.php'); + + exit; + } + api_tree_move_node(grv('tree_id'), grv('id'), grv('parent'), grv('position')); break; case 'rename_node': + if (!is_tree_allowed((int) grv('tree_id'))) { + raise_message('tree_idor', __('You do not have permission to modify this tree.'), MESSAGE_LEVEL_ERROR); + header('Location: tree.php'); + + exit; + } + api_tree_rename_node(grv('tree_id'), grv('id'), gnrv('text')); break; @@ -1265,7 +1300,7 @@ function tree_edit(bool $partial = false) : void { } }) .on('create_node.jstree', function (e, data) { - $.get('?action=create_node', { 'id' : data.node.parent, 'tree_id' : $('#id').val(), 'position' : data.position, 'text' : data.node.text }) + $.post('?action=create_node', { 'id' : data.node.parent, 'tree_id' : $('#id').val(), 'position' : data.position, 'text' : data.node.text, '__csrf_magic' : csrfMagicToken }) .done(function (d) { data.instance.set_id(data.node, d.id); data.instance.set_text(data.node, d.text); @@ -1281,7 +1316,7 @@ function tree_edit(bool $partial = false) : void { }); }) .on('rename_node.jstree', function (e, data) { - $.get('?action=rename_node', { 'id' : data.node.id, 'tree_id' : $('#id').val(), 'text' : data.text }) + $.post('?action=rename_node', { 'id' : data.node.id, 'tree_id' : $('#id').val(), 'text' : data.text, '__csrf_magic' : csrfMagicToken }) .done(function (d) { if (d.result == 'false') { data.instance.set_text(data.node, d.text); @@ -1296,7 +1331,7 @@ function tree_edit(bool $partial = false) : void { }); }) .on('move_node.jstree', function (e, data) { - $.get('?action=move_node', { 'id' : data.node.id, 'tree_id' : $('#id').val(), 'parent' : data.parent, 'position' : data.position }) + $.post('?action=move_node', { 'id' : data.node.id, 'tree_id' : $('#id').val(), 'parent' : data.parent, 'position' : data.position, '__csrf_magic' : csrfMagicToken }) .always(function () { var st = data.instance.get_state(); data.instance.load_node(data.instance.get_parent(data.node.id), function () { this.set_state(st); }); @@ -1313,7 +1348,7 @@ function tree_edit(bool $partial = false) : void { $('#graphs').jstree().deselect_all(); } - $.get('?action=copy_node', { 'id' : data.original.id, 'tree_id' : $('#id').val(), 'parent' : data.parent, 'position' : data.position }) + $.post('?action=copy_node', { 'id' : data.original.id, 'tree_id' : $('#id').val(), 'parent' : data.parent, 'position' : data.position, '__csrf_magic' : csrfMagicToken }) .always(function () { var st = data.instance.get_state(); data.instance.load_node(data.instance.get_parent(data.node.id), function () { this.set_state(st); });