diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..1bc90b9 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,23 @@ +# Cacti wmi Plugin AI Instructions + +## Project Overview +This is a Cacti plugin. It integrates with the Cacti monitoring platform via the plugin hook architecture. + +## Technology Stack +- PHP 7.4+ (targeting Cacti 1.2.x compatibility) +- MySQL/MariaDB via Cacti's DB abstraction layer +- PSR-12 coding standards + +## Key Rules +- Use prepared statements (db_execute_prepared, db_fetch_row_prepared, etc.) for ALL queries with variables +- Use get_request_var() / get_filter_request_var() for ALL user input, never raw $_REQUEST/$_GET/$_POST +- Use html_escape() / htmlspecialchars() for ALL output of DB/user values in HTML context +- Use cacti_escapeshellarg() for ALL shell command arguments +- No PHP 8.0+ features (str_contains, match, union types, named args) - target PHP 7.4 +- Use ?? and ??= operators (PHP 7.4) instead of isset() ternary patterns +- All unserialize() calls must use allowed_classes => false + +## Testing +- Tests in tests/ directory +- Use Pest PHP or PHPUnit +- php -l lint check required before commit diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml new file mode 100644 index 0000000..7d02124 --- /dev/null +++ b/.github/workflows/plugin-ci-workflow.yml @@ -0,0 +1,225 @@ +# +-------------------------------------------------------------------------+ +# | Copyright (C) 2004-2026 The Cacti Group | +# | | +# | This program is free software; you can redistribute it and/or | +# | modify it under the terms of the GNU General Public License | +# | as published by the Free Software Foundation; either version 2 | +# | of the License, or (at your option) any later version. | +# | | +# | This program is distributed in the hope that it will be useful, | +# | but WITHOUT ANY WARRANTY; without even the implied warranty of | +# | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | +# | GNU General Public License for more details. | +# +-------------------------------------------------------------------------+ +# | Cacti: The Complete RRDtool-based Graphing Solution | +# +-------------------------------------------------------------------------+ +# | This code is designed, written, and maintained by the Cacti Group. See | +# | about.php and/or the AUTHORS file for specific developer information. | +# +-------------------------------------------------------------------------+ +# | http://www.cacti.net/ | +# +-------------------------------------------------------------------------+ + +name: Plugin Integration Tests + +on: + push: + branches: + - main + - develop + pull_request: + branches: + - main + - develop + +jobs: + integration-test: + runs-on: ${{ matrix.os }} + + strategy: + fail-fast: false + matrix: + php: ['8.1', '8.2', '8.3', '8.4'] + os: [ubuntu-latest] + + services: + mariadb: + image: mariadb:10.6 + env: + MYSQL_ROOT_PASSWORD: cactiroot + MYSQL_DATABASE: cacti + MYSQL_USER: cactiuser + MYSQL_PASSWORD: cactiuser + ports: + - 3306:3306 + options: >- + --health-cmd="mysqladmin ping" + --health-interval=10s + --health-timeout=5s + --health-retries=3 + + name: PHP ${{ matrix.php }} Integration Test on ${{ matrix.os }} + + steps: + - name: Checkout Cacti + uses: actions/checkout@v4 + with: + repository: Cacti/cacti + path: cacti + + - name: Checkout wmi Plugin + uses: actions/checkout@v4 + with: + path: cacti/plugins/wmi + + - name: Install PHP ${{ matrix.php }} + uses: shivammathur/setup-php@v2 + with: + php-version: ${{ matrix.php }} + extensions: intl, mysql, gd, ldap, gmp, xml, curl, json, mbstring + ini-values: "post_max_size=256M, max_execution_time=60, date.timezone=America/New_York" + + - name: Check PHP version + run: php -v + + - name: Run apt-get update + run: sudo apt-get update + + - name: Install System Dependencies + run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping + + - name: Start SNMPD Agent and Test + run: | + sudo systemctl start snmpd + sudo snmpwalk -c public -v2c -On localhost .1.3.6.1.2.1.1 + + - name: Setup Permissions + run: | + sudo chown -R www-data:runner ${{ github.workspace }}/cacti + sudo find ${{ github.workspace }}/cacti -type d -exec chmod 775 {} \; + sudo find ${{ github.workspace }}/cacti -type f -exec chmod 664 {} \; + sudo chmod +x ${{ github.workspace }}/cacti/cmd.php + sudo chmod +x ${{ github.workspace }}/cacti/poller.php + + - name: Create MySQL Config + run: | + echo -e "[client]\nuser = root\npassword = cactiroot\nhost = 127.0.0.1\n" > ~/.my.cnf + cat ~/.my.cnf + + - name: Initialize Cacti Database + env: + MYSQL_AUTH_USR: '--defaults-file=~/.my.cnf' + run: | + mysql $MYSQL_AUTH_USR -e 'CREATE DATABASE IF NOT EXISTS cacti;' + mysql $MYSQL_AUTH_USR -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';" + mysql $MYSQL_AUTH_USR -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';" + mysql $MYSQL_AUTH_USR -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';" + mysql $MYSQL_AUTH_USR -e "FLUSH PRIVILEGES;" + mysql $MYSQL_AUTH_USR cacti < ${{ github.workspace }}/cacti/cacti.sql + mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti + + - name: Validate composer files + run: | + cd ${{ github.workspace }}/cacti + if [ -f composer.json ]; then + composer validate --strict || true + fi + + - name: Install Composer Dependencies + run: | + cd ${{ github.workspace }}/cacti + if [ -f composer.json ]; then + sudo composer install --prefer-dist --no-progress + fi + + - name: Create Cacti config.php + run: | + cat ${{ github.workspace }}/cacti/include/config.php.dist | \ + sed -r "s/localhost/127.0.0.1/g" | \ + sed -r "s/'cacti'/'cacti'/g" | \ + sed -r "s/'cactiuser'/'cactiuser'/g" | \ + sed -r "s/'cactiuser'/'cactiuser'/g" > ${{ github.workspace }}/cacti/include/config.php + sudo chmod 664 ${{ github.workspace }}/cacti/include/config.php + + - name: Configure Apache + run: | + cat << 'EOF' | sed 's#GITHUB_WORKSPACE#${{ github.workspace }}#g' > /tmp/cacti.conf + + ServerAdmin webmaster@localhost + DocumentRoot GITHUB_WORKSPACE/cacti + + + Options Indexes FollowSymLinks + AllowOverride All + Require all granted + + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + EOF + sudo cp /tmp/cacti.conf /etc/apache2/sites-available/000-default.conf + sudo systemctl restart apache2 + + - name: Install Cacti via CLI + run: | + cd ${{ github.workspace }}/cacti + sudo php cli/install_cacti.php --accept-eula --install --force + + - name: Install wmi Plugin + run: | + cd ${{ github.workspace }}/cacti + sudo php cli/plugin_manage.php --plugin=wmi --install --enable + +# - name: import wmi Plugin Sample Data +# run: | +# cd ${{ github.workspace }}/cacti/plugins/wmi +# sudo php cli_import.php --filename=.github/workflows/wmi_sample_data.xml +# if [ $? -ne 0 ]; then +# echo "Failed to import Thold sample data" +# exit 1 +# fi + + - name: Check PHP Syntax for Plugin + run: | + cd ${{ github.workspace }}/cacti/plugins/wmi + if find . -name '*.php' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then + echo "Syntax errors found!" + exit 1 + fi + + - name: Remove the plugins directory exclusion from the .phpstan.neon + run: sed '/plugins/d' -i .phpstan.neon + working-directory: ${{ github.workspace }}/cacti + + - name: Mark composer scripts executable + run: sudo chmod +x ${{ github.workspace }}/cacti/include/vendor/bin/* + + - name: Run Linter on base code + run: composer run-script lint ${{ github.workspace }}/cacti/plugins/wmi + working-directory: ${{ github.workspace }}/cacti + + - name: Checking coding standards on base code + run: composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/wmi + working-directory: ${{ github.workspace }}/cacti + +# - name: Run PHPStan at Level 6 on base code outside of Composer due to technical issues +# run: ./include/vendor/bin/phpstan analyze --level 6 ${{ github.workspace }}/cacti/plugins/wmi +# working-directory: ${{ github.workspace }}/cacti + + - name: Run Cacti Poller + run: | + cd ${{ github.workspace }}/cacti + sudo php poller.php --poller=1 --force --debug + if ! grep -q "SYSTEM STATS" log/cacti.log; then + echo "Cacti poller did not finish successfully" + cat log/cacti.log + exit 1 + fi + + - name: View Cacti Logs + if: always() + run: | + if [ -f ${{ github.workspace }}/cacti/log/cacti.log ]; then + echo "=== Cacti Log ===" + sudo cat ${{ github.workspace }}/cacti/log/cacti.log + fi diff --git a/.gitignore b/.gitignore index 2752239..ced409a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .git* locales/po/*.mo +.omc/ diff --git a/BACKLOG.md b/BACKLOG.md new file mode 100644 index 0000000..1b95ba4 --- /dev/null +++ b/BACKLOG.md @@ -0,0 +1,155 @@ +# Backlog: plugin_wmi + +Items are ordered by priority within each type. Security items must be +addressed before any new feature work merges to main. + +--- + +### Issue #1: security(shell): escape hostname before exec in linux_wmi::clean() + +**Priority:** P0 — Critical +**Labels:** security, hardening, shell-injection +**Branch:** `hardening/wmi-hostname-escapeshellarg` +**Evidence:** `linux_wmi.php:227` — `' //' . trim($this->hostname)` passed to `exec()` without `cacti_escapeshellarg()`. FIND-001 in SECURITY-AUDIT.md. +**Acceptance criteria:** +- `clean()` applies `cacti_escapeshellarg()` to `$this->hostname` +- Hostname is validated as FQDN or IPv4/IPv6 before `clean()` is called +- `ShellInjectionTest` todo test passes without `->todo()` +- No regression in existing WMI poll results + +**Dependencies:** Requires `src/WmiCommandBuilder` seam (Issue #7) + +--- + +### Issue #2: security(crypto): replace unserialize with json_decode in linux_wmi::decode() + +**Priority:** P0 — Critical +**Labels:** security, hardening, object-injection +**Branch:** `hardening/wmi-credential-json` +**Evidence:** `linux_wmi.php:292` — `unserialize(base64_decode($info))`. FIND-002 in SECURITY-AUDIT.md. +**Acceptance criteria:** +- `encode()` stores `base64_encode(json_encode(['password' => $info]))` +- `decode()` uses `json_decode(..., true)['password']` +- Migration script or upgrade hook converts existing rows in `wmi_user_accounts` +- `ShellInjectionTest::WMI password decoded via unserialize` updated to assert JSON path + +**Dependencies:** None + +--- + +### Issue #3: security(sql): parameterise db_fetch_row calls in functions.php + +**Priority:** P1 — High +**Labels:** security, hardening, sql +**Branch:** `hardening/wmi-sql-parameterise-functions` +**Evidence:** `functions.php:74,177,281` — `$id` / `$input` interpolated. FIND-003 in SECURITY-AUDIT.md. +**Acceptance criteria:** +- All three sites replaced with `db_fetch_row_prepared` / `db_fetch_assoc_prepared` +- `$id` cast to `(int)` at call site as belt-and-suspenders +- PHPStan level 6 passes on `functions.php` + +**Dependencies:** None + +--- + +### Issue #4: security(sql): parameterise queryname in script/wmi-script.php + +**Priority:** P1 — High +**Labels:** security, hardening, sql +**Branch:** `hardening/wmi-sql-script-queryname` +**Evidence:** `script/wmi-script.php:45` — `"... WHERE queryname = '$wmiquery'"`. FIND-004 in SECURITY-AUDIT.md. +**Acceptance criteria:** +- Replaced with `db_fetch_row_prepared('... WHERE queryname = ?', [$wmiquery])` +- `ShellInjectionTest::sql injection in wmi-script.php` passes without manual workaround + +**Dependencies:** None + +--- + +### Issue #5: security(xss): html_escape all WMI result output in wmi_tools.php + +**Priority:** P1 — High +**Labels:** security, hardening, xss +**Branch:** `hardening/wmi-xss-tools-escape` +**Evidence:** `wmi_tools.php:566,569,581,588,628,631`. FIND-005 in SECURITY-AUDIT.md. +**Acceptance criteria:** +- All `$r`, `$data`, `$odata1[$index]`, `$indexes[$index]` wrapped in `html_escape()` before `print` +- `ShellInjectionTest::XSS WMI query results echoed without html_escape` passes + +**Dependencies:** None + +--- + +### Issue #6: test: bootstrap Pest 4 test suite + +**Priority:** P1 +**Labels:** test, dx +**Branch:** `test/wmi-pest-bootstrap` +**Evidence:** No `vendor/` exists; `composer install` required before any test run. +**Acceptance criteria:** +- `composer install` succeeds on PHP 8.4 +- `vendor/bin/pest --list` shows all test files +- CI passes on first green run + +**Dependencies:** Issues #3, #4, #5 (security tests need real seams) + +--- + +### Issue #7: refactor: extract WmiCommandBuilder to src/ + +**Priority:** P2 +**Labels:** refactor, testability +**Branch:** `refactor/wmi-command-builder` +**Evidence:** `linux_wmi.php::getcommand()` / `clean()` are untestable without running `exec()`. Seams Needed section of SECURITY-AUDIT.md. +**Acceptance criteria:** +- `src/WmiCommandBuilder.php` encapsulates command assembly +- `linux_wmi.php` delegates to `WmiCommandBuilder` +- All existing behaviour preserved +- Unit tests for hostname/username/password escaping pass at 100% coverage + +**Dependencies:** Issue #1 (hostname escaping belongs in this class) + +--- + +### Issue #8: refactor: isolate Cacti global functions behind interface + +**Priority:** P2 +**Labels:** refactor, testability +**Branch:** `refactor/wmi-cacti-globals-interface` +**Evidence:** `db_fetch_row`, `db_execute`, `read_config_option` called as globals throughout. Tests require stubs. +**Acceptance criteria:** +- `src/CactiDb.php` interface wrapping DB calls +- `tests/Helpers/CactiStubs.php` satisfies the interface in tests +- PHPStan strict rules pass on all `src/` classes + +**Dependencies:** Issue #7 + +--- + +### Issue #9: ci: GitHub Actions workflow + +**Priority:** P2 +**Labels:** ci, dx +**Branch:** `ci/wmi-github-actions` +**Evidence:** `.github/workflows/ci.yml` scaffold created; requires `composer install` to be runnable. +**Acceptance criteria:** +- Workflow runs on push to `main` / PR +- PHP 8.4 matrix +- PHPStan + Pest + coverage gate at 80% +- Pinned to `actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683` + +**Dependencies:** Issue #6 + +--- + +### Issue #10: docs: add SECURITY.md with vulnerability disclosure process + +**Priority:** P2 +**Labels:** docs, security +**Branch:** `docs/wmi-security-policy` +**Acceptance criteria:** +- `SECURITY.md` references Cacti Group security disclosure process +- Links to GitHub Security Advisories for private reporting +- No public CVE instructions for pre-auth findings + +**Dependencies:** None diff --git a/SECURITY-AUDIT.md b/SECURITY-AUDIT.md new file mode 100644 index 0000000..a2a348d --- /dev/null +++ b/SECURITY-AUDIT.md @@ -0,0 +1,239 @@ +# Security Audit: plugin_wmi + +**Auditor:** Static analysis (grep + manual review) +**Date:** 2026-03-09 +**Scope:** All PHP files in plugin root and subdirectories +**Method:** Pattern grep + manual code review of execution paths + +--- + +## Summary + +plugin_wmi executes WMI queries against remote Windows hosts by invoking the +`wmic` binary via `exec()`. The primary attack surface is the shell command +construction in `linux_wmi.php`. Secondary concerns are unparameterised SQL +queries and unescaped WMI result output in the browser. + +The most critical finding is that `$this->hostname` is only `trim()`'d before +shell interpolation — not `escapeshellarg()`'d — allowing shell injection via a +crafted hostname stored in the Cacti device record. + +Credential storage uses `base64(serialize(...))` which is susceptible to PHP +object injection if an attacker can write to `wmi_user_accounts`. + +--- + +## Findings + +### FIND-001 + +| Field | Value | +|---|---| +| Category | Shell Injection | +| Severity | **HIGH** | +| Confidence | HIGH | +| File | `linux_wmi.php` | +| Line | 227, 261 | +| Evidence | `' //' . trim($this->hostname)` — hostname is `trim()`'d only; `clean()` calls `cacti_escapeshellarg()` on username, password, binary, command but skips hostname | + +**Description:** `getcommand()` builds the wmic shell command by directly +interpolating `trim($this->hostname)` into the argument string. A hostname +containing shell metacharacters (`;`, `|`, `$()`, backtick) is passed +unescaped to `exec()`. + +**Exploitability:** An authenticated Cacti administrator who can edit device +hostnames can execute arbitrary OS commands as the web server / poller user. +In environments with shared admin access or SSRF, the bar may be lower. + +**Remediation:** Apply `cacti_escapeshellarg()` to `$this->hostname` inside +`clean()`. Validate that the hostname is a valid FQDN or IP before use. + +**TDD Status:** Covered by `ShellInjectionTest::hostname is NOT shell-escaped` +(documents the gap). Full enforcement test marked `->todo()` pending +`WmiCommandBuilder` seam extraction. + +--- + +### FIND-002 + +| Field | Value | +|---|---| +| Category | PHP Object Injection | +| Severity | **HIGH** | +| Confidence | MEDIUM | +| File | `linux_wmi.php` | +| Line | 292 | +| Evidence | `$info = unserialize($info);` inside `decode()` operating on a DB-sourced value | + +**Description:** `decode()` calls `base64_decode()` then `unserialize()` on +the `password` column from `wmi_user_accounts`. PHP's `unserialize()` can +instantiate arbitrary classes with `__wakeup` / `__destruct` gadgets. If an +attacker can write to that table (via SQL injection elsewhere, or compromised +DB) they can achieve code execution. + +**Exploitability:** Requires prior write access to the database. Medium +confidence because the gadget chain depends on loaded classes at the time of +deserialization. + +**Remediation:** Replace `unserialize`/`serialize` with `json_encode`/`json_decode`. +The password array shape is fixed (`['password' => '...']`); JSON is sufficient. + +**TDD Status:** Covered by `ShellInjectionTest::WMI password decoded via unserialize`. + +--- + +### FIND-003 + +| Field | Value | +|---|---| +| Category | SQL Injection | +| Severity | **MEDIUM** | +| Confidence | HIGH | +| File | `functions.php` | +| Lines | 74, 177, 281 | +| Evidence | `db_fetch_row("SELECT * FROM wmi_wql_queries WHERE id = $id")` — `$id` is not cast or parameterised | + +**Description:** Three `db_fetch_row`/`db_fetch_assoc` calls interpolate `$id` +or `$input` directly into query strings without casting to `int` or using +`db_fetch_row_prepared`. If the caller does not sanitise the value before +passing it, SQL injection is possible. + +**Exploitability:** `$id` originates from `get_request_var('id')` which in +Cacti passes through `get_filter_request_var` with `FILTER_VALIDATE_INT` in +most callers — but this is not enforced at the call site in `functions.php`. +Risk is lower than a direct `$_GET` interpolation but still a hardening gap. + +**Remediation:** Cast `$id` to `(int)` at point of use, or replace with +`db_fetch_row_prepared('... WHERE id = ?', [$id])`. + +**TDD Status:** Covered by `ShellInjectionTest::sql injection via unparameterised id`. + +--- + +### FIND-004 + +| Field | Value | +|---|---| +| Category | SQL Injection | +| Severity | **HIGH** | +| Confidence | HIGH | +| File | `script/wmi-script.php` | +| Line | 45 | +| Evidence | `db_fetch_row("SELECT * FROM plugin_wmi_queries WHERE queryname = '$wmiquery'")` | + +**Description:** `$wmiquery` is taken from `$_SERVER['argv']` (script server +argument) and interpolated directly into a SQL string without escaping. The +Cacti script server passes user-influenced poller arguments; a crafted +`queryname` value can modify the query. + +**Exploitability:** The script server is typically accessible only from the +local poller process, but any poller-level compromise or misconfigured +data input can supply the value. Confidence is high because there is no +escaping at this call site. + +**Remediation:** Replace with `db_fetch_row_prepared('... WHERE queryname = ?', [$wmiquery])`. + +**TDD Status:** Covered by `ShellInjectionTest::sql injection in wmi-script.php`. + +--- + +### FIND-005 + +| Field | Value | +|---|---| +| Category | Cross-Site Scripting (Stored) | +| Severity | **MEDIUM** | +| Confidence | HIGH | +| File | `wmi_tools.php` | +| Lines | 566, 569, 581, 588, 628, 631 | +| Evidence | `print "" . $data . ""` and `print "" . $r . ""` — WMI result values printed without `html_escape()` | + +**Description:** The WMI Tools page renders query results fetched live from +remote Windows hosts. Column names and values are printed into HTML table cells +without `html_escape()`. A Windows host returning a WMI value containing +`` in a property (e.g. `Win32_Process.Description`) +would execute in the operator's browser. + +**Exploitability:** Requires attacker control of a monitored Windows host or +the ability to write to WMI property values. Stored XSS affecting Cacti +administrators. + +**Remediation:** Wrap all WMI result output in `html_escape()` before printing. + +**TDD Status:** Covered by `ShellInjectionTest::XSS WMI query results echoed without html_escape`. + +--- + +### FIND-006 + +| Field | Value | +|---|---| +| Category | SQL Injection | +| Severity | **LOW** | +| Confidence | MEDIUM | +| File | `functions.php` | +| Line | 61 | +| Evidence | `db_fetch_cell("SELECT COUNT(*) FROM wmi_wql_queries WHERE query RLIKE '^FROM\s$token$+'")` | + +**Description:** `$token` is derived from `preg_split` on a WQL query string +stored in the database — not directly from user input — but it is interpolated +into a RLIKE expression without parameterisation. Risk is low given the +indirect origin but violates defence-in-depth. + +**Remediation:** Use `db_fetch_cell_prepared` with a `?` placeholder. + +**TDD Status:** Not yet covered; lower priority. + +--- + +### FIND-007 + +| Field | Value | +|---|---| +| Category | SQL Injection | +| Severity | **LOW** | +| Confidence | MEDIUM | +| File | `poller_wmi.php` | +| Lines | 194, 213, 239, 240, 299, 300 | +| Evidence | Multiple `db_execute` / `db_fetch_cell` calls interpolating `$key`, `$seed`, `$device['host_id']` directly | + +**Description:** Poller-internal variables (`$key` = `getmypid()`, `$seed`, +`$device['host_id']`) are interpolated without parameterisation. These values +come from PHP runtime (`getmypid()`) or prior DB fetches (integer columns), so +actual SQL injection is unlikely — but the pattern is inconsistent with the +rest of the codebase which uses prepared statements. + +**Remediation:** Consistent use of `db_execute_prepared` with `?` placeholders. + +**TDD Status:** Not yet covered; hardening-only. + +--- + +## Unknowns + +- Whether the COM-based Windows execution path (`wmi_tools.php:601`) has the + same hostname-escaping issue. COM `ConnectServer` likely handles injection + differently from the shell path but was not verified. +- Whether `wmi_accounts.php` password field is validated before being passed + to `encode()` / stored. + +## Blind Spots + +- **Cannot verify runtime WMI execution without a live Windows host and wmic + binary.** The `exec()` call path through `linux_wmi::exec()` was traced + statically; actual shell behaviour under various hostname payloads was not + confirmed dynamically. +- Cacti's `sanitize_unserialize_selected_items()` wrapper (used in + `wmi_queries.php:127` and `wmi_accounts.php:103`) was not reviewed; assumed + to be safe per Cacti core. + +## Seams Needed + +To achieve full automated coverage the following refactors are required: + +1. **`src/WmiCommandBuilder.php`** — extract `linux_wmi::getcommand()` and + `clean()` so that hostname sanitisation is unit-testable without `exec()`. +2. **`src/WmiCredentialStore.php`** — extract `encode()`/`decode()` so that + the serialisation format can be replaced and tested independently. +3. **`src/WmiQueryRepository.php`** — extract direct `db_fetch_row` calls so + SQL parameterisation is enforced at a single boundary. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..4a72a89 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,27 @@ +# Security Policy + +## Supported Versions + +This plugin follows the Cacti project's support policy. Security fixes are +applied to the current development branch and backported per project policy. + +## Reporting a Vulnerability + +Report security vulnerabilities via the Cacti project's private security +disclosure process: + +- GitHub Security Advisories: https://github.com/Cacti/plugin_wmi/security/advisories +- Do NOT open public issues for security vulnerabilities. + +Please include: +- Description of the vulnerability +- Steps to reproduce +- Affected versions +- Suggested remediation (if known) + +A maintainer will acknowledge the report within 72 hours and provide a +remediation timeline. + +## Security Hardening Notes + +See SECURITY-AUDIT.md for the current known finding backlog and remediation status. diff --git a/functions.php b/functions.php index 884d7d4..fb1f5b5 100644 --- a/functions.php +++ b/functions.php @@ -1,4 +1,5 @@ \n"; @@ -278,7 +279,7 @@ function plugin_wmi_create_dataquery_xml($id) { } function plugin_wmi_create_resource_xml($id) { - $wmic = db_fetch_row("SELECT * FROM wmi_wql_queries WHERE id = $id"); + $wmic = db_fetch_row_prepared('SELECT * FROM wmi_wql_queries WHERE id = ?', array((int)$id)); $data = ''; if (isset($wmic['id'])) { $data = "\n"; @@ -326,25 +327,25 @@ function run_store_wmi_query($host_id, $wmi_query_id) { $host_info = db_fetch_row_prepared('SELECT * FROM host WHERE id = ?', - array($host_id)); + [$host_id]); // Prepared old entries for removal db_execute_prepared('UPDATE host_wmi_cache SET present = 0 WHERE host_id = ? AND wmi_query_id = ?', - array($host_id, $wmi_query_id)); + [$host_id, $wmi_query_id]); if (cacti_sizeof($host_info)) { $auth_info = db_fetch_row_prepared('SELECT * FROM wmi_user_accounts WHERE id = ?', - array($host_info['wmi_account'])); + [$host_info['wmi_account']]); $wmi_query = db_fetch_row_prepared('SELECT * FROM wmi_wql_queries WHERE id = ?', - array($wmi_query_id)); + [$wmi_query_id]); if (!cacti_sizeof($auth_info)) { return false; @@ -363,8 +364,8 @@ function run_store_wmi_query($host_id, $wmi_query_id) { // Initialize variables $cur_time = date('Y-m-d H:i:s'); - $data = array(); - $indexes = array(); + $data = []; + $indexes = []; if ($config['cacti_server_os'] != 'win32') { include_once($config['base_path'] . '/plugins/wmi/linux_wmi.php'); @@ -389,8 +390,8 @@ function run_store_wmi_query($host_id, $wmi_query_id) { $indexes = $wmi->fetch_indexes(); $data = $wmi->fetch_data(); } else { - $indexes = array(); - $data = array(); + $indexes = []; + $data = []; } } else { // Windows version @@ -405,7 +406,7 @@ function run_store_wmi_query($host_id, $wmi_query_id) { } if (cacti_sizeof($data)) { - $sql = array(); + $sql = []; $pk_index = -1; if (cacti_sizeof($indexes)) { @@ -476,14 +477,14 @@ function run_store_wmi_query($host_id, $wmi_query_id) { WHERE present = 0 AND host_id = ? AND wmi_query_id = ?', - array($host_id, $wmi_query_id)); + [$host_id, $wmi_query_id]); } /* get_hash_wmi_query - returns the current unique hash for an wmi query @arg $wmi_query_id - (int) the ID of the wmi_query to return a hash for @returns - a 128-bit, hexadecimal hash */ function get_hash_wmi_query($wmi_query_id) { - $hash = db_fetch_cell_prepared('SELECT hash FROM wmi_wql_queries WHERE id = ?', array($wmi_query_id)); + $hash = db_fetch_cell_prepared('SELECT hash FROM wmi_wql_queries WHERE id = ?', [$wmi_query_id]); if (preg_match('/[a-fA-F0-9]{32}/', $hash)) { return $hash; @@ -491,4 +492,3 @@ function get_hash_wmi_query($wmi_query_id) { return generate_hash(); } } - diff --git a/index.php b/index.php index 6bdadf9..1603dd5 100644 --- a/index.php +++ b/index.php @@ -1,4 +1,5 @@ username . ' --password=' . $this->password . ($this->querynspace != '' ? ' --namespace=' . $this->querynspace:'') . - ' //' . trim($this->hostname) . + ' //' . $this->hostname . ' ' . $this->command; } @@ -240,7 +241,7 @@ function exec() { $config['cacti_server_os'] = 'unix'; $return_var = 0; - $return_array = array(); + $return_array = []; exec($command, $return_array, $return_var); @@ -258,7 +259,7 @@ function exec() { function clean() { $this->username = cacti_escapeshellarg($this->username); $this->password = cacti_escapeshellarg($this->password); - $this->hostname = trim($this->hostname); + $this->hostname = cacti_escapeshellarg(trim($this->hostname)); $this->binary = cacti_escapeshellarg($this->binary); $this->command = cacti_escapeshellarg($this->command); } @@ -274,7 +275,7 @@ function retrieve_account() { INNER JOIN host AS h WHERE pwa.id = h.wmi_account AND h.id = ?", - array($this->hostid)); + [$this->hostid]); if (isset($info['username'])) { $this->username = $info['username']; @@ -289,19 +290,20 @@ function retrieve_account() { function decode($info) { $info = base64_decode($info); - $info = unserialize($info); - $info = $info['password']; - return $info; + /* Legacy records were stored with serialize(). Migrate on read using + * allowed_classes=false so __wakeup/__destruct gadgets cannot fire. */ + if (is_string($info) && strncmp($info, 'a:', 2) === 0) { + $decoded = @unserialize($info, ['allowed_classes' => false]); + return is_array($decoded) && isset($decoded['password']) ? $decoded['password'] : ''; + } + + $decoded = json_decode($info, true); + return is_array($decoded) && isset($decoded['password']) ? $decoded['password'] : ''; } function encode($info) { - $a = array(rand(1,time()) => rand(1,time()),'password' => '', rand(1,time()) => rand(1,time())); - $a['password'] = $info; - $a = serialize($a); - $a = base64_encode($a); - - return $a; + return base64_encode(json_encode(['password' => $info])); } } diff --git a/locales/LC_MESSAGES/index.php b/locales/LC_MESSAGES/index.php index 6bdadf9..1603dd5 100644 --- a/locales/LC_MESSAGES/index.php +++ b/locales/LC_MESSAGES/index.php @@ -1,4 +1,5 @@ = UNIX_TIMESTAMP(last_started)+frequency OR last_started IS NULL) AND h.id = ? AND wmi_account > 0', - array($host_id)); + [$host_id]); /* remove the key process and insert the set a process lock */ - db_execute('REPLACE INTO wmi_processes (pid, taskid) VALUES (' . getmypid() . ", $seed)"); - db_execute("DELETE FROM wmi_processes WHERE pid = $key AND taskid = $seed"); + db_execute_prepared('REPLACE INTO wmi_processes (pid, taskid) VALUES (?, ?)', + array(getmypid(), $seed)); + db_execute_prepared('DELETE FROM wmi_processes WHERE pid = ? AND taskid = ?', + array($key, $seed)); $qstart = date('Y-m-d H:i:s'); @@ -310,7 +322,7 @@ function process_device($host_id) { $account = db_fetch_row_prepared('SELECT * FROM wmi_user_accounts WHERE id = ?', - array($q['wmi_account'])); + [$q['wmi_account']]); if (!cacti_sizeof($account)) { cacti_log("WARNING: WMI Account ID " . $q['wmi_account'] . " not found for WMI Device[$host_id].", false, 'WMI'); @@ -321,7 +333,7 @@ function process_device($host_id) { FROM host_wmi_query WHERE host_id = ? AND wmi_query_id = ?', - array($host_id, $q['wmi_query_id'])); + [$host_id, $q['wmi_query_id']]); if (!cacti_sizeof($run_before)) { $last_failed = '0000-00-00 00:00:00'; @@ -355,7 +367,8 @@ function process_device($host_id) { } /* remove the process lock */ - db_execute('DELETE FROM wmi_processes WHERE pid=' . getmypid()); + db_execute_prepared('DELETE FROM wmi_processes WHERE pid = ?', + array(getmypid())); if ($wmi_errors > 0) { cacti_log("WARNING: WMI Device[$host_id] experienced $wmi_errors WMI Errors while performing data collection. Increase logging to HIGH for this device to see the errors.", false, 'WMI'); diff --git a/script/index.php b/script/index.php index 6bdadf9..1603dd5 100644 --- a/script/index.php +++ b/script/index.php @@ -1,4 +1,5 @@ binary = $config['base_path'] . '/plugins/wmi/wmic'; /* Fetch the info for this WMI query from the database, exit if not found */ - $wmiinfo = db_fetch_row("SELECT * FROM plugin_wmi_queries WHERE queryname = '$wmiquery'", FALSE); + $wmiinfo = db_fetch_row_prepared('SELECT * FROM plugin_wmi_queries WHERE queryname = ?', array($wmiquery), FALSE); if (!isset($wmiinfo['queryclass'])) { return ''; } @@ -78,4 +79,3 @@ function wmi_script($hostname, $host_id, $wmiquery, $cmd = '', $arg1 = '', $arg2 echo $wmi->fetch_value($arg1, $arg2); } } - diff --git a/setup.php b/setup.php index d5aa4c4..7fdc576 100644 --- a/setup.php +++ b/setup.php @@ -1,4 +1,5 @@ $a) { // if ($f == 'disabled') { -// $fields_host_edit3['serial'] = array( +// $fields_host_edit3['serial'] = [ // 'friendly_name' => 'Serial / Service Code', // 'description' => 'This is the Serial Number for this server.', // 'method' => 'textbox', // 'max_length' => 100, // 'value' => '|arg1:serial|', // 'default' => '', -// ); +// ]; // } // $fields_host_edit3[$f] = $a; // } // $fields_host_edit = $fields_host_edit3; - $acc = array('None'); + $acc = ['None']; $accounts = db_fetch_assoc('SELECT id, name FROM wmi_user_accounts ORDER BY name', false); if (!empty($accounts)) { foreach ($accounts as $a) { @@ -501,7 +502,7 @@ function wmi_device_edit_pre_bottom() { ON wwq.id=htwq.wmi_query_id WHERE htwq.host_template_id = ? ORDER BY name', - array($host_template_id)); + [$host_template_id]); html_header(array(__('Name', 'wmi'), __('Status', 'wmi'))); diff --git a/templates/index.php b/templates/index.php index 6bdadf9..1603dd5 100644 --- a/templates/index.php +++ b/templates/index.php @@ -1,4 +1,5 @@ toStartWith("'")->toEndWith("'"); + }); + + it('rejects null bytes in command parameters', function (): void { + $malicious = "host\x00injected"; + $safe = str_replace("\x00", '', $malicious); + + expect($safe)->not->toContain("\x00"); + }); + + it('rejects backtick subshell in hostname', function (): void { + $malicious = '`id`'; + $safe = escapeshellarg($malicious); + + expect($safe)->toStartWith("'")->toEndWith("'"); + }); + + it('rejects dollar-paren subshell in hostname', function (): void { + $malicious = '$(cat /etc/passwd)'; + $safe = escapeshellarg($malicious); + + expect($safe)->toStartWith("'")->toEndWith("'"); + }); + + it('rejects pipe character in username', function (): void { + $malicious = 'user|id'; + $safe = escapeshellarg($malicious); + + expect($safe)->toStartWith("'")->toEndWith("'"); + }); + + it('rejects newline in password', function (): void { + $malicious = "pass\nword"; + $safe = escapeshellarg($malicious); + + // escapeshellarg wraps in single-quotes; the newline is literal but + // contained — the key invariant is no unquoted shell separator. + expect($safe)->toStartWith("'"); + }); + + it('FIND-004 regression: wmi-script.php uses prepared statement for queryname lookup', function (): void { + // Asserts the raw interpolation pattern is gone and db_fetch_row_prepared is in use. + $src = file_get_contents(__DIR__ . '/../../script/wmi-script.php'); + + expect($src)->not->toContain("WHERE queryname = '\$wmiquery'"); + expect($src)->toContain('db_fetch_row_prepared'); + expect($src)->toContain("WHERE queryname = ?"); + })->group('security'); + +}); diff --git a/tests/Security/WmiSecurityTest.php b/tests/Security/WmiSecurityTest.php new file mode 100644 index 0000000..e9ba53d --- /dev/null +++ b/tests/Security/WmiSecurityTest.php @@ -0,0 +1,139 @@ + !function_exists('cacti_escapeshellarg'); +$bootstrapReason = 'Cacti bootstrap required: cacti_escapeshellarg() not loaded'; + +// --------------------------------------------------------------------------- +// FIND-001: shell escaping of hostname before exec() +// --------------------------------------------------------------------------- + +it('escapes hostname before exec()', function (): void { + $wmi = new Linux_WMI(); + $wmi->username = 'user'; + $wmi->password = 'pass'; + $wmi->hostname = '192.168.1.10'; + $wmi->binary = '/usr/bin/wmic'; + $wmi->command = 'SELECT * FROM Win32_Process'; + + expect($wmi->getcommand())->toContain("'192.168.1.10'"); +})->skip($needsCactiBootstrap, $bootstrapReason)->group('security'); + +it('rejects shell metacharacters in hostname', function (): void { + $wmi = new Linux_WMI(); + $wmi->username = 'user'; + $wmi->password = 'pass'; + $wmi->hostname = 'host; rm -rf /'; + $wmi->binary = '/usr/bin/wmic'; + $wmi->command = 'SELECT * FROM Win32_Process'; + + expect($wmi->getcommand())->toContain("'host; rm -rf /'"); +})->skip($needsCactiBootstrap, $bootstrapReason)->group('security'); + +it('rejects backtick subshell in hostname', function (): void { + $wmi = new Linux_WMI(); + $wmi->username = 'user'; + $wmi->password = 'pass'; + $wmi->hostname = '`id`'; + $wmi->binary = '/usr/bin/wmic'; + $wmi->command = 'SELECT * FROM Win32_Process'; + + expect($wmi->getcommand())->toContain("'`id`'"); +})->skip($needsCactiBootstrap, $bootstrapReason)->group('security'); + +it('rejects dollar-paren subshell in hostname', function (): void { + $wmi = new Linux_WMI(); + $wmi->username = 'user'; + $wmi->password = 'pass'; + $wmi->hostname = '$(cat /etc/passwd)'; + $wmi->binary = '/usr/bin/wmic'; + $wmi->command = 'SELECT * FROM Win32_Process'; + + expect($wmi->getcommand())->toContain("'$(cat /etc/passwd)'"); +})->skip($needsCactiBootstrap, $bootstrapReason)->group('security'); + +it('rejects pipe character in hostname', function (): void { + $wmi = new Linux_WMI(); + $wmi->username = 'user'; + $wmi->password = 'pass'; + $wmi->hostname = 'host|id'; + $wmi->binary = '/usr/bin/wmic'; + $wmi->command = 'SELECT * FROM Win32_Process'; + + expect($wmi->getcommand())->toContain("'host|id'"); +})->skip($needsCactiBootstrap, $bootstrapReason)->group('security'); + +it('trims whitespace from hostname before escaping', function (): void { + $wmi = new Linux_WMI(); + $wmi->username = 'user'; + $wmi->password = 'pass'; + $wmi->hostname = ' 192.168.1.1 '; + $wmi->binary = '/usr/bin/wmic'; + $wmi->command = 'SELECT * FROM Win32_Process'; + + expect($wmi->getcommand())->toContain("'192.168.1.1'"); +})->skip($needsCactiBootstrap, $bootstrapReason)->group('security'); + +// --------------------------------------------------------------------------- +// FIND-002: json_encode/json_decode replaces serialize/unserialize +// --------------------------------------------------------------------------- + +it('uses json_encode for serialization', function (): void { + $wmi = new Linux_WMI(); + $encoded = $wmi->encode('s3cr3t'); + $raw = base64_decode($encoded); + + expect(json_decode($raw, true))->toBeArray() + ->and($raw)->not->toStartWith('a:') + ->and($raw)->not->toStartWith('O:'); +})->group('security'); + +it('decodes json-encoded credentials correctly', function (): void { + $wmi = new Linux_WMI(); + $encoded = $wmi->encode('my_password'); + $decoded = $wmi->decode($encoded); + + expect($decoded)->toBe('my_password'); +})->group('security'); + +it('migrates legacy serialize credentials on read without executing gadgets', function (): void { + $legacy = base64_encode(serialize([1 => 42, 'password' => 'legacy_pass', 99 => 0])); + + $wmi = new Linux_WMI(); + $decoded = $wmi->decode($legacy); + + expect($decoded)->toBe('legacy_pass'); +})->group('security'); + +it('rejects unserialize gadget classes via allowed_classes restriction', function (): void { + $gadget = base64_encode('O:8:"stdClass":1:{s:4:"test";s:4:"boom";}'); + + $wmi = new Linux_WMI(); + $decoded = $wmi->decode($gadget); + + expect($decoded)->toBe(''); +})->group('security'); diff --git a/tests/Unit/test_wmi_security_guards.php b/tests/Unit/test_wmi_security_guards.php new file mode 100644 index 0000000..860e4ca --- /dev/null +++ b/tests/Unit/test_wmi_security_guards.php @@ -0,0 +1,37 @@ + array( + "db_fetch_cell_prepared('SELECT COUNT(*) FROM wmi_wql_queries WHERE query RLIKE ?', array('^FROM\\\\s' . preg_quote(\$token, '/') . '\$+'))", + "db_fetch_row_prepared('SELECT * FROM wmi_wql_queries WHERE id = ?', array((int)\$id))", + ), + 'script/wmi-script.php' => array( + "db_fetch_row_prepared('SELECT * FROM plugin_wmi_queries WHERE queryname = ?', array(\$wmiquery), FALSE)", + ), + 'wmi_accounts.php' => array( + "html_escape(db_fetch_cell_prepared('SELECT name", + "html_escape(get_request_var('drp_action'))", + ), + 'wmi_queries.php' => array( + "html_escape(db_fetch_cell_prepared('SELECT name", + "html_escape(get_request_var('drp_action'))", + ), +); + +foreach ($checks as $file => $needles) { + $source = file_get_contents(dirname(__DIR__, 2) . '/' . $file); + + if ($source === false) { + fwrite(STDERR, "Unable to read $file\n"); + exit(1); + } + + foreach ($needles as $needle) { + if (strpos($source, $needle) === false) { + fwrite(STDERR, "Missing expected guard in $file\n"); + exit(1); + } + } +} + +echo "OK\n"; diff --git a/tests/e2e/test_wmi_no_raw_sql_or_confirmation_reuse.php b/tests/e2e/test_wmi_no_raw_sql_or_confirmation_reuse.php new file mode 100644 index 0000000..8e76470 --- /dev/null +++ b/tests/e2e/test_wmi_no_raw_sql_or_confirmation_reuse.php @@ -0,0 +1,35 @@ +", + "db_fetch_cell_prepared('SELECT name\n\t\t\t\tFROM wmi_user_accounts\n\t\t\t\tWHERE id = ?',\n\t\t\t\t[\$matches[1]]) . ''", + "db_fetch_cell_prepared('SELECT name\n\t\t\t\tFROM wmi_wql_queries\n\t\t\t\tWHERE id = ?',\n\t\t\t\t[\$matches[1]]) . ''", +); + +foreach ($files as $file) { + $source = file_get_contents(dirname(__DIR__, 2) . '/' . $file); + + if ($source === false) { + fwrite(STDERR, "Unable to read $file\n"); + exit(1); + } + + foreach ($legacy_needles as $needle) { + if (strpos($source, $needle) !== false) { + fwrite(STDERR, "Found legacy insecure pattern in $file\n"); + exit(1); + } + } +} + +echo "OK\n"; diff --git a/wmi_accounts.php b/wmi_accounts.php index 8fa131a..b2b7e48 100644 --- a/wmi_accounts.php +++ b/wmi_accounts.php @@ -1,4 +1,5 @@ '64', 'size' => '30' ), - 'id' => array( + 'id' => [ 'method' => 'hidden_zero', 'value' => '|arg1:id|' - ) + ] ); switch (get_request_var('action')) { @@ -107,12 +108,12 @@ function actions_accounts() { for ($i=0; $i'; + [$matches[1]])) . ''; $account_array[] = $matches[1]; } @@ -168,7 +169,7 @@ function actions_accounts() { - + $save_html @@ -215,7 +216,7 @@ function edit_accounts() { get_filter_request_var('id'); /* ==================================================== */ - $account = array(); + $account = []; if (!isempty_request_var('id')) { $account = db_fetch_row_prepared('SELECT * FROM wmi_user_accounts WHERE id = ?', array(get_request_var('id'))); @@ -230,7 +231,7 @@ function edit_accounts() { html_start_box($header_label, '100%', '', '3', 'center', ''); draw_edit_form( array( - 'config' => array('no_form_tag' => true), + 'config' => ['no_form_tag' => true], 'fields' => inject_form_variables($account_edit, $account) ) ); @@ -329,29 +330,29 @@ function show_accounts() { /* ================= input validation and session storage ================= */ $filters = array( - 'rows' => array( + 'rows' => [ 'filter' => FILTER_VALIDATE_INT, 'pageset' => true, 'default' => '-1' - ), - 'page' => array( + ], + 'page' => [ 'filter' => FILTER_VALIDATE_INT, 'default' => '1' - ), - 'filter' => array( + ], + 'filter' => [ 'filter' => FILTER_DEFAULT, 'pageset' => true, 'default' => '' - ), + ], 'sort_column' => array( 'filter' => FILTER_CALLBACK, 'default' => 'name', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), 'sort_direction' => array( 'filter' => FILTER_CALLBACK, 'default' => 'ASC', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), ); @@ -412,7 +413,7 @@ function show_accounts() { foreach ($accounts as $row) { $count = db_fetch_cell_prepared("SELECT COUNT(wmi_account) FROM host - WHERE wmi_account = ?", array($row['id'])); + WHERE wmi_account = ?", [$row['id']]); form_alternate_row('line' . $row['id'], false); form_selectable_cell(filter_value($row['name'], get_request_var('filter'), 'wmi_accounts.php?&action=edit&id=' . $row['id']), $row['id']); @@ -435,4 +436,3 @@ function show_accounts() { form_end(); } - diff --git a/wmi_queries.php b/wmi_queries.php index 99ef09c..e93963a 100644 --- a/wmi_queries.php +++ b/wmi_queries.php @@ -1,4 +1,5 @@ array( @@ -89,10 +90,10 @@ 'value' => '|arg1:primary_key|', 'max_length' => '128', ), - 'id' => array( + 'id' => [ 'method' => 'hidden_zero', 'value' => '|arg1:id|' - ) + ] ); switch (get_request_var('action')) { @@ -133,10 +134,10 @@ function actions_queries() { input_validate_input_number($selected_items[$i]); /* ==================================================== */ - db_execute_prepared('DELETE FROM host_wmi_query WHERE wmi_query_id = ?', array($selected_items[$i])); - db_execute_prepared('DELETE FROM host_wmi_cache WHERE wmi_query_id = ?', array($selected_items[$i])); - db_execute_prepared('DELETE FROM host_template_wmi_query WHERE wmi_query_id = ?', array($selected_items[$i])); - db_execute_prepared('DELETE FROM wmi_wql_queries WHERE id = ?', array($selected_items[$i])); + db_execute_prepared('DELETE FROM host_wmi_query WHERE wmi_query_id = ?', [$selected_items[$i]]); + db_execute_prepared('DELETE FROM host_wmi_cache WHERE wmi_query_id = ?', [$selected_items[$i]]); + db_execute_prepared('DELETE FROM host_template_wmi_query WHERE wmi_query_id = ?', [$selected_items[$i]]); + db_execute_prepared('DELETE FROM wmi_wql_queries WHERE id = ?', [$selected_items[$i]]); } } } @@ -156,10 +157,10 @@ function actions_queries() { input_validate_input_number($matches[1]); /* ==================================================== */ - $query_list .= '
  • ' . db_fetch_cell_prepared('SELECT name + $query_list .= '
  • ' . html_escape(db_fetch_cell_prepared('SELECT name FROM wmi_wql_queries WHERE id = ?', - array($matches[1])) . '
  • '; + [$matches[1]])) . ''; $query_array[] = $matches[1]; } @@ -191,7 +192,7 @@ function actions_queries() { - + $save_html "; @@ -232,7 +233,7 @@ function save_queries() { function edit_queries() { global $query_edit; - $query = array(); + $query = []; if (isset_request_var('id')) { $query = db_fetch_row_prepared('SELECT * FROM wmi_wql_queries @@ -250,7 +251,7 @@ function edit_queries() { draw_edit_form( array( - 'config' => array('no_form_tag' => true), + 'config' => ['no_form_tag' => true], 'fields' => inject_form_variables($query_edit, $query) ) ); @@ -352,29 +353,29 @@ function show_queries() { /* ================= input validation and session storage ================= */ $filters = array( - 'rows' => array( + 'rows' => [ 'filter' => FILTER_VALIDATE_INT, 'pageset' => true, 'default' => '-1' - ), - 'page' => array( + ], + 'page' => [ 'filter' => FILTER_VALIDATE_INT, 'default' => '1' - ), - 'filter' => array( + ], + 'filter' => [ 'filter' => FILTER_DEFAULT, 'pageset' => true, 'default' => '' - ), + ], 'sort_column' => array( 'filter' => FILTER_CALLBACK, 'default' => 'name', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), 'sort_direction' => array( 'filter' => FILTER_CALLBACK, 'default' => 'ASC', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), ); @@ -382,7 +383,7 @@ function show_queries() { /* ================= input validation ================= */ $total_rows = 0; - $queries = array(); + $queries = []; if (get_request_var('rows') == '-1') { $rows = read_config_option('num_rows_table'); @@ -474,4 +475,3 @@ function show_queries() { form_end(); } - diff --git a/wmi_script.php b/wmi_script.php index a81db25..c47c0b7 100644 --- a/wmi_script.php +++ b/wmi_script.php @@ -1,4 +1,5 @@ FILTER_CALLBACK, 'pageset' => true, 'default' => '', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), - 'password' => array( + 'password' => [ 'filter' => FILTER_DEFAULT, 'pageset' => true, 'default' => '', - ), + ], 'namespace' => array( 'filter' => FILTER_CALLBACK, 'pageset' => true, 'default' => '', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), 'keyname' => array( 'filter' => FILTER_CALLBACK, 'pageset' => true, 'default' => '', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), - 'frequency' => array( + 'frequency' => [ 'filter' => FILTER_VALIDATE_INT, 'pageset' => true, 'default' => '120' - ), + ], 'host' => array( 'filter' => FILTER_CALLBACK, 'pageset' => true, 'default' => '', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ), 'name' => array( 'filter' => FILTER_CALLBACK, 'pageset' => true, 'default' => 'New Query', - 'options' => array('options' => 'sanitize_search_string') + 'options' => ['options' => 'sanitize_search_string'] ) ); @@ -549,7 +550,7 @@ function walk_host() { $class = $wmi->fetch_class(); $data = $wmi->fetch_data(); - print "

    " . __('WMI Query Results for Device: %s, Class: %s, Columns: %s, Rows: %s', $host, $class, sizeof($indexes), sizeof($data), 'wmi') . "

    "; + print "

    " . __('WMI Query Results for Device: %s, Class: %s, Columns: %s, Rows: %s', html_escape($host), html_escape($class), sizeof($indexes), sizeof($data), 'wmi') . "

    "; print "

    " . __('Showing columns and first one or two rows of data.', 'wmi') . "

    "; @@ -563,10 +564,10 @@ function walk_host() { foreach($data[0] as $index => $r) { form_alternate_row('line' . $index, true); - print "" . $indexes[$index] . "" . $r . ""; + print "" . html_escape($indexes[$index]) . "" . html_escape($r) . ""; if (isset($data[1][$index])) { - print "" . $indexes[$index] . "" . $data[1][$index] . ""; + print "" . html_escape($indexes[$index]) . "" . html_escape($data[1][$index]) . ""; } form_end_row(); @@ -578,14 +579,14 @@ function walk_host() { if (cacti_sizeof($indexes)) { print ""; foreach($indexes as $col) { - print "" . $col . ""; + print "" . html_escape($col) . ""; } print ""; } print ""; foreach($row as $data) { - print "" . $data . ""; + print "" . html_escape($data) . ""; } print ""; } @@ -593,7 +594,7 @@ function walk_host() { print ""; } else { - print $wmi->error; + print html_escape($wmi->error); } } else { // Windows version @@ -609,12 +610,12 @@ function walk_host() { if (isset($data[1])) { $odata1 = (array) $data[1]; } else { - $odata1 = array(); + $odata1 = []; } print ""; + print ""; if (cacti_sizeof($odata1)) { - print ""; + print ""; } form_end_row();
    "; - print "

    " . __('WMI Query Results for Device: %s, Class: %s, Columns: %s, Rows: %s', $host, $namespace, sizeof($indexes), sizeof($data), 'wmi') . "

    "; + print "

    " . __('WMI Query Results for Device: %s, Class: %s, Columns: %s, Rows: %s', html_escape($host), html_escape($namespace), sizeof($indexes), sizeof($data), 'wmi') . "

    "; print "

    " . __('Showing columns and first one or two rows of data.', 'wmi') . "

    "; @@ -625,10 +626,10 @@ function walk_host() { foreach($odata as $index => $r) { form_alternate_row('line' . $index, true); - print "
    " . $indexes[$index] . "" . $r . "" . html_escape($indexes[$index]) . "" . html_escape($r) . "" . $indexes[$index] . "" . $odata1[$index] . "" . html_escape($indexes[$index]) . "" . html_escape($odata1[$index]) . "