diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index e806000..02b1688 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -85,7 +85,7 @@ jobs: run: sudo apt-get update - name: Install System Dependencies - run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }} + run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping - name: Start SNMPD Agent and Test run: | diff --git a/.gitignore b/.gitignore index 69f2746..d03590e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ configs/** output/** +.omc/ diff --git a/BACKLOG.md b/BACKLOG.md new file mode 100644 index 0000000..539b550 --- /dev/null +++ b/BACKLOG.md @@ -0,0 +1,117 @@ +# Backlog: plugin_weathermap + +Issues derived from SECURITY-AUDIT.md. Do not push to GitHub until contrib-ledger gate passes. + +--- + +## Issue #1: refactor: extract FpingRunner and RrdCommandBuilder seams +**Priority**: medium (blocker for security tests) +**Labels**: refactor, tech-debt +**Branch**: `refactor/1-shell-command-seams` +**Evidence**: WeatherMapDataSource_fping.php:103; WeatherMapDataSource_rrd.php:296–325 +**Acceptance criteria**: +- [ ] `src/Shell/FpingRunner.php` wraps popen call; accepts pre-built command string +- [ ] `src/Rrd/RrdCommandBuilder.php` assembles rrdtool command string from typed args +- [ ] Both classes instantiable without Cacti bootstrap +- [ ] PHPStan level 6 passes on `src/` + +**Dependencies**: none + +--- + +## Issue #2: security(cmd): escapeshellarg fping target and validate as IP/hostname +**Priority**: medium +**Labels**: security +**Branch**: `security/2-escapeshellarg-fping-target` +**Evidence**: lib/datasources/WeatherMapDataSource_fping.php:103 (WM-CMD-01) +**Acceptance criteria**: +- [ ] `$target` validated with `filter_var($target, FILTER_VALIDATE_IP)` or strict hostname regex before use +- [ ] `escapeshellarg($target)` applied in command string +- [ ] `src/Shell/FpingRunner.php` seam extracted and unit-tested +- [ ] Security test WM-CMD-01 passes (todo removed) +- [ ] Targets that fail IP/hostname validation are rejected with a logged warning + +**Dependencies**: Issue #1 + +--- + +## Issue #3: security(cmd): fix RRD popen argument quoting +**Priority**: low +**Labels**: security, tech-debt +**Branch**: `security/3-fix-rrd-arg-quoting` +**Evidence**: lib/datasources/WeatherMapDataSource_rrd.php:325, 429 (WM-CMD-02) +**Acceptance criteria**: +- [ ] Manual double-quote wrapping replaced with `escapeshellarg()` per argument +- [ ] `$rrdfile` validated as absolute filesystem path with no shell metacharacters +- [ ] `src/Rrd/RrdCommandBuilder.php` seam extracted and unit-tested +- [ ] Security test WM-CMD-02 passes (todo removed) + +**Dependencies**: Issue #1 + +--- + +## Issue #4: security(path): add backslash check to wm_editor_sanitize_conffile +**Priority**: low +**Labels**: security, tech-debt +**Branch**: `security/4-sanitize-conffile-backslash` +**Evidence**: lib/editor.inc.php:232–246 (WM-PATH-01) +**Acceptance criteria**: +- [ ] `wm_editor_sanitize_conffile` rejects filenames containing `\` +- [ ] Path traversal test WM-PATH-01 (backslash case) todo removed +- [ ] Existing positive test cases remain green + +**Dependencies**: none + +--- + +## Issue #5: security(input): validate $action against allowed-list in editor +**Priority**: medium +**Labels**: security, tech-debt +**Branch**: `security/5-validate-editor-action` +**Evidence**: weathermap-cacti-plugin-editor.php:91 — `$action = get_nfilter_request_var('action')` without allowed-list validation +**Acceptance criteria**: +- [ ] `$action` validated against an explicit list of valid editor action strings +- [ ] Unknown actions produce a logged warning and abort +- [ ] PHPStan level 6 passes + +**Dependencies**: none + +--- + +## Issue #6: refactor: extract MapnameValidator as testable class +**Priority**: low +**Labels**: refactor, tech-debt +**Branch**: `refactor/6-mapname-validator-class` +**Evidence**: lib/editor.inc.php:232–246 +**Acceptance criteria**: +- [ ] `src/Config/MapnameValidator.php` wraps `wm_editor_sanitize_conffile` logic +- [ ] All existing sanitization cases covered by unit tests +- [ ] PHPStan level 6 passes + +**Dependencies**: none + +--- + +## Issue #7: ci: add Pest 4 + PHPStan CI workflow +**Priority**: medium +**Labels**: ci +**Branch**: `ci/7-pest-phpstan-workflow` +**Acceptance criteria**: +- [ ] `.github/workflows/test.yml` runs `composer test` and `composer analyse` +- [ ] PHPStan level 6 clean on `src/` and `tests/` +- [ ] Actions pinned to full commit SHA + +**Dependencies**: Issues #1–#5 + +--- + +## Issue #8: docs: complete audit of weathermap-cacti-plugin-editor.php action dispatch +**Priority**: medium +**Labels**: security, docs +**Branch**: `docs/8-editor-action-audit` +**Evidence**: editor action variable not fully traced in this audit +**Acceptance criteria**: +- [ ] All action values and their code paths documented +- [ ] Any state-changing actions that lack CSRF or auth checks identified and filed as separate issues + +**Dependencies**: none diff --git a/SECURITY-AUDIT.md b/SECURITY-AUDIT.md new file mode 100644 index 0000000..f12100c --- /dev/null +++ b/SECURITY-AUDIT.md @@ -0,0 +1,174 @@ +# Security Audit: plugin_weathermap + +**Date**: 2026-03-09 +**Auditor**: automated static analysis + manual review +**Scope**: all PHP files under plugin_weathermap/ excluding vendor/ (none present) + +--- + +## Summary + +Weathermap is the most security-mature of the four plugins audited. The Cacti Group +rewrite uses prepared statements throughout the management UI (weathermap-cacti-plugin-mgmt.php), +consistently applies html_escape() on DB-sourced output, and enforces api_plugin_user_realm_auth +at well-defined entry points. The two remaining surface areas are in the map-rendering engine +(datasource plugins) where admin-controlled config values flow into shell commands without +escapeshellarg, and a residual Windows-only path traversal gap in the config filename sanitizer. + +--- + +## Findings + +### WM-CMD-01 — OS Command Injection via map config TARGET value (MEDIUM) +**File**: `lib/datasources/WeatherMapDataSource_fping.php:103` +**Category**: OS Command Injection +**Severity**: MEDIUM (requires write access to a .conf map file; admin-controlled input) + +```php +$command = $this->fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1"; +$pipe = popen($command, 'r'); +``` + +`$target` is extracted from the TARGET line in a weathermap `.conf` file via the regex +`/^fping:(\S+)$/`. The regex captures any non-whitespace string, including shell +metacharacters (`;`, `|`, `&`, `` ` ``, `$`). A crafted TARGET line such as +`TARGET fping:192.0.2.1;id>/tmp/x` injects into the shell command. + +**Attack path**: Admin writes (or attacker achieves file-write on) a .conf file containing +a malicious TARGET line → weathermap poller processes it → `popen` executes the injected command +as the web/poller process user. + +**Remediation**: Wrap `$target` with `escapeshellarg()`. Additionally, validate that +`$target` is a valid IP address or hostname before use (reject if it fails `filter_var` +with `FILTER_VALIDATE_IP` or a strict hostname regex). + +--- + +### WM-CMD-02 — OS Command Injection via RRD file path in popen command (LOW-MEDIUM) +**File**: `lib/datasources/WeatherMapDataSource_rrd.php:325, 429` +**Category**: OS Command Injection +**Severity**: LOW-MEDIUM (rrdfile sourced from Cacti DB, admin-controlled) + +```php +// args array assembled with: +$args[] = "DEF:in=$rrdfile:" . $dsnames[IN] . ":$cf"; +// then: +foreach ($args as $arg) { + if (strchr($arg, ' ') != false) { + $command .= ' "' . $arg . '"'; + } else { + $command .= ' ' . $arg; + } +} +$pipe = popen($command, 'r'); +``` + +`$rrdfile` and `$dsnames` originate from Cacti's `data_template_data` table (admin-set +RRD paths). The quoting logic uses double quotes around args containing spaces, but does +not escape double-quote characters within the value itself. A `$rrdfile` containing `"` +or `` ` `` breaks out of the double-quote boundary. + +The `db_fetch_row_prepared` calls that source `$rrdfile` use parameterized queries, so +the DB-layer is safe. The risk is an admin storing a crafted RRD path. + +**Remediation**: Validate `$rrdfile` as an absolute filesystem path with no shell +metacharacters before embedding in command args. Use `escapeshellarg()` per argument +rather than manual double-quoting. + +--- + +### WM-PATH-01 — Path traversal in map config filename (LOW — mitigated, residual Windows risk) +**File**: `weathermap-cacti-plugin-editor.php:96`, `lib/editor.inc.php:232–246` +**Category**: Path Traversal +**Severity**: LOW (effectively mitigated on Linux/macOS; residual risk on Windows) + +```php +function wm_editor_sanitize_conffile($filename) { + $filename = wm_editor_sanitize_uri($filename); + if (substr($filename, -5, 5) != '.conf') { $filename = ''; } + if (strstr($filename, '/') !== false) { $filename = ''; } + return $filename; +} +``` + +The sanitizer enforces `.conf` extension and rejects any `/` character, preventing +Unix path traversal (e.g., `../../etc/passwd.conf` is rejected). PHP null-byte injection +(`file.conf\0.php`) was fixed in PHP 8.0; with PHP 8.4 as the requirement it is not +exploitable. + +Residual gap: Windows path separator `\` is not checked. On Windows hosts, +`..\\configs\\other.conf` passes both checks. Since Cacti targets Linux, this is +informational. + +**Remediation**: Add a backslash check for defense-in-depth: `if (strstr($filename, '\\') !== false) { $filename = ''; }`. + +--- + +### WM-POSITIVE-01 — Management UI uses prepared statements throughout (INFORMATIONAL) +**File**: `weathermap-cacti-plugin-mgmt.php:354–390, 467–476` + +The `weathermap_form_actions()` function iterates `$_POST`, extracts checkbox keys via +`preg_match('/^chk_([0-9:a-z]+)$/', ...)`, validates numeric parts with +`input_validate_input_number()`, and uses `db_execute_prepared` with `?` placeholders +for all INSERT/DELETE operations. DB-sourced output is wrapped with `html_escape()`. +No SQL injection or XSS issues found in this file. + +--- + +### WM-POSITIVE-02 — Auth enforcement is consistent (INFORMATIONAL) +**File**: `weathermap-cacti-plugin.php:418, 445`, `setup.php:766` + +`api_plugin_user_realm_auth` is called at the correct entry points for both viewer and +management realms. No unauthenticated paths to state-modifying actions were found. + +--- + +### WM-POSITIVE-03 — Editor mapname sanitization prevents path traversal (INFORMATIONAL) +**File**: `weathermap-cacti-plugin-editor.php:91–96` + +```php +$mapname = get_nfilter_request_var('mapname'); +$mapname = wm_editor_sanitize_conffile($mapname); +``` + +The sanitizer is applied immediately after reading `$mapname`. The `wm_editor_sanitize_uri` +call within `wm_editor_sanitize_conffile` additionally strips dangerous URI characters. +This is a well-implemented control. + +--- + +## Unknowns and Blind Spots + +- **weathermap-cacti-plugin-editor.php** was only partially read. The editor has a large + surface area (map config editing, node/link add/remove). The `$action` variable is read + via `get_nfilter_request_var('action')` without validation against an allowed-list; + tracing what each action value triggers requires a full read. +- **`lib/editor.actions.php`** was not fully read. It uses `wm_editor_sanitize_conffile` + on `$sourcemapname` (line 68) which is correct, but other action parameters may not + be sanitized. +- **`weathermap-cacti-rebuild.php`** was not examined. Rebuild scripts that process + map configs offline may have additional shell execution paths. +- **`check.php`** was not examined. +- **fping target IP validation**: whether `$target` is validated as a valid IP/hostname + anywhere before reaching the popen call was not confirmed beyond the `\S+` regex. + +--- + +## Seams Required Before TDD Can Fully Proceed + +| Seam | Files Affected | Description | +|------|---------------|-------------| +| `src/Shell/FpingRunner.php` | WeatherMapDataSource_fping.php:103 | Extract popen call; test command assembly without executing fping | +| `src/Rrd/RrdCommandBuilder.php` | WeatherMapDataSource_rrd.php:296–325 | Extract arg assembly; test escapeshellarg application | +| `src/Config/MapnameValidator.php` | editor.inc.php:232–246 | Expose sanitize logic as a testable class method | + +--- + +## Estimated Effort + +| Finding | Fix Effort | +|---------|-----------| +| WM-CMD-01 (fping escapeshellarg + IP validation) | 1 h | +| WM-CMD-02 (rrd arg quoting) | 1 h | +| WM-PATH-01 (backslash check) | 15 min | +| Full test coverage via seams | 1 day | diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..a1a8a46 --- /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_weathermap/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/check.php b/check.php index 30a82a5..c5f7dd1 100644 --- a/check.php +++ b/check.php @@ -1,4 +1,6 @@ 2, 'max_bandwidth_out_cfg'=>3]], - ['LINK', '/^\s*(MAXVALUE|BANDWIDTH)\s+(\d+\.?\d*[KMGT]?)\s*$/i', ['max_bandwidth_in_cfg'=>2, 'max_bandwidth_out_cfg'=>2]], - ['NODE', '/^\s*(MAXVALUE)\s+(\d+\.?\d*[KMGT]?)\s+(\d+\.?\d*[KMGT]?)\s*$/i', ['max_bandwidth_in_cfg'=>2, 'max_bandwidth_out_cfg'=>3]], - ['NODE', '/^\s*(MAXVALUE)\s+(\d+\.?\d*[KMGT]?)\s*$/i', ['max_bandwidth_in_cfg'=>2, 'max_bandwidth_out_cfg'=>2]], - ['GLOBAL', '/^\s*BACKGROUND\s+(.*)\s*$/i', ['background'=>1]], - ['GLOBAL', '/^\s*HTMLOUTPUTFILE\s+(.*)\s*$/i', ['htmloutputfile'=>1]], - ['GLOBAL', '/^\s*HTMLSTYLESHEET\s+(.*)\s*$/i', ['htmlstylesheet'=>1]], - ['GLOBAL', '/^\s*IMAGEOUTPUTFILE\s+(.*)\s*$/i', ['imageoutputfile'=>1]], - ['GLOBAL', '/^\s*DATAOUTPUTFILE\s+(.*)\s*$/i', ['dataoutputfile'=>1]], - ['GLOBAL', '/^\s*IMAGEURI\s+(.*)\s*$/i', ['imageuri'=>1]], - ['GLOBAL', '/^\s*TITLE\s+(.*)\s*$/i', ['title'=>1]], - ['GLOBAL', '/^\s*HTMLSTYLE\s+(static|overlib)\s*$/i', ['htmlstyle'=>1]], - ['GLOBAL', '/^\s*KEYFONT\s+(\d+)\s*$/i', ['keyfont'=>1]], - ['GLOBAL', '/^\s*TITLEFONT\s+(\d+)\s*$/i', ['titlefont'=>1]], - ['GLOBAL', '/^\s*TIMEFONT\s+(\d+)\s*$/i', ['timefont'=>1]], - ['GLOBAL', '/^\s*TITLEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['titlex'=>1, 'titley'=>2]], - ['GLOBAL', '/^\s*TITLEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['titlex'=>1, 'titley'=>2, 'title'=>3]], - ['GLOBAL', '/^\s*TIMEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['timex'=>1, 'timey'=>2]], - ['GLOBAL', '/^\s*TIMEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['timex'=>1, 'timey'=>2, 'stamptext'=>3]], - ['GLOBAL', '/^\s*MINTIMEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['mintimex'=>1, 'mintimey'=>2]], - ['GLOBAL', '/^\s*MINTIMEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['mintimex'=>1, 'mintimey'=>2, 'minstamptext'=>3]], - ['GLOBAL', '/^\s*MAXTIMEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['maxtimex'=>1, 'maxtimey'=>2]], - ['GLOBAL', '/^\s*MAXTIMEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['maxtimex'=>1, 'maxtimey'=>2, 'maxstamptext'=>3]], - ['NODE', "/^\s*LABEL\s*$/i", ['label'=>'']], // special case for blank labels - ['NODE', "/^\s*LABEL\s+(.*)\s*$/i", ['label'=>1]], - ['(LINK|GLOBAL)', "/^\s*WIDTH\s+(\d+)\s*$/i", ['width'=>1]], - ['(LINK|GLOBAL)', "/^\s*HEIGHT\s+(\d+)\s*$/i", ['height'=>1]], - ['LINK', "/^\s*WIDTH\s+(\d+\.\d+)\s*$/i", ['width'=>1]], - ['LINK', '/^\s*ARROWSTYLE\s+(classic|compact)\s*$/i', ['arrowstyle'=>1]], - ['LINK', '/^\s*VIASTYLE\s+(curved|angled)\s*$/i', ['viastyle'=>1]], - ['LINK', '/^\s*INCOMMENT\s+(.*)\s*$/i', ['comments[IN]'=>1]], - ['LINK', '/^\s*OUTCOMMENT\s+(.*)\s*$/i', ['comments[OUT]'=>1]], - ['LINK', '/^\s*BWFONT\s+(\d+)\s*$/i', ['bwfont'=>1]], - ['LINK', '/^\s*COMMENTFONT\s+(\d+)\s*$/i', ['commentfont'=>1]], - ['LINK', '/^\s*COMMENTSTYLE\s+(edge|center)\s*$/i', ['commentstyle'=>1]], - ['LINK', '/^\s*DUPLEX\s+(full|half)\s*$/i', ['duplex'=>1]], - ['LINK', '/^\s*BWSTYLE\s+(classic|angled)\s*$/i', ['labelboxstyle'=>1]], - ['LINK', '/^\s*LINKSTYLE\s+(twoway|oneway)\s*$/i', ['linkstyle'=>1]], - ['LINK', '/^\s*BWLABELPOS\s+(\d+)\s(\d+)\s*$/i', ['labeloffset_in'=>1, 'labeloffset_out'=>2]], - ['LINK', '/^\s*COMMENTPOS\s+(\d+)\s(\d+)\s*$/i', ['commentoffset_in'=>1, 'commentoffset_out'=>2]], - ['LINK', '/^\s*USESCALE\s+([A-Za-z][A-Za-z0-9_]*)\s*$/i', ['usescale'=>1]], - ['LINK', '/^\s*USESCALE\s+([A-Za-z][A-Za-z0-9_]*)\s+(absolute|percent)\s*$/i', ['usescale'=>1, 'scaletype'=>2]], - - ['LINK', '/^\s*SPLITPOS\s+(\d+)\s*$/i', ['splitpos'=>1]], - - ['NODE', '/^\s*LABELOFFSET\s+([-+]?\d+)\s+([-+]?\d+)\s*$/i', ['labeloffsetx'=>1, 'labeloffsety'=>2]], - ['NODE', '/^\s*LABELOFFSET\s+(C|NE|SE|NW|SW|N|S|E|W)\s*$/i', ['labeloffset'=>1]], - ['NODE', '/^\s*LABELOFFSET\s+((C|NE|SE|NW|SW|N|S|E|W)\d+)\s*$/i', ['labeloffset'=>1]], - ['NODE', '/^\s*LABELOFFSET\s+(-?\d+r\d+)\s*$/i', ['labeloffset'=>1]], - - ['NODE', '/^\s*LABELFONT\s+(\d+)\s*$/i', ['labelfont'=>1]], - ['NODE', '/^\s*LABELANGLE\s+(0|90|180|270)\s*$/i', ['labelangle'=>1]], + ['LINK', '/^\s*(MAXVALUE|BANDWIDTH)\s+(\d+\.?\d*[KMGT]?)\s*$/i', ['max_bandwidth_in_cfg' => 2, 'max_bandwidth_out_cfg'=>2]], + ['NODE', '/^\s*(MAXVALUE)\s+(\d+\.?\d*[KMGT]?)\s+(\d+\.?\d*[KMGT]?)\s*$/i', ['max_bandwidth_in_cfg'=> 2, 'max_bandwidth_out_cfg'=>3]], + ['NODE', '/^\s*(MAXVALUE)\s+(\d+\.?\d*[KMGT]?)\s*$/i', ['max_bandwidth_in_cfg' => 2, 'max_bandwidth_out_cfg'=>2]], + ['GLOBAL', '/^\s*BACKGROUND\s+(.*)\s*$/i', ['background' => 1]], + ['GLOBAL', '/^\s*HTMLOUTPUTFILE\s+(.*)\s*$/i', ['htmloutputfile' => 1]], + ['GLOBAL', '/^\s*HTMLSTYLESHEET\s+(.*)\s*$/i', ['htmlstylesheet' => 1]], + ['GLOBAL', '/^\s*IMAGEOUTPUTFILE\s+(.*)\s*$/i', ['imageoutputfile' => 1]], + ['GLOBAL', '/^\s*DATAOUTPUTFILE\s+(.*)\s*$/i', ['dataoutputfile' => 1]], + ['GLOBAL', '/^\s*IMAGEURI\s+(.*)\s*$/i', ['imageuri' => 1]], + ['GLOBAL', '/^\s*TITLE\s+(.*)\s*$/i', ['title' => 1]], + ['GLOBAL', '/^\s*HTMLSTYLE\s+(static|overlib)\s*$/i', ['htmlstyle' => 1]], + ['GLOBAL', '/^\s*KEYFONT\s+(\d+)\s*$/i', ['keyfont' => 1]], + ['GLOBAL', '/^\s*TITLEFONT\s+(\d+)\s*$/i', ['titlefont' => 1]], + ['GLOBAL', '/^\s*TIMEFONT\s+(\d+)\s*$/i', ['timefont' => 1]], + ['GLOBAL', '/^\s*TITLEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['titlex' => 1, 'titley'=>2]], + ['GLOBAL', '/^\s*TITLEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['titlex' => 1, 'titley'=>2, 'title'=>3]], + ['GLOBAL', '/^\s*TIMEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['timex' => 1, 'timey'=>2]], + ['GLOBAL', '/^\s*TIMEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['timex' => 1, 'timey'=>2, 'stamptext'=>3]], + ['GLOBAL', '/^\s*MINTIMEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['mintimex' => 1, 'mintimey'=>2]], + ['GLOBAL', '/^\s*MINTIMEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['mintimex' => 1, 'mintimey'=>2, 'minstamptext'=>3]], + ['GLOBAL', '/^\s*MAXTIMEPOS\s+(-?\d+)\s+(-?\d+)\s*$/i', ['maxtimex' => 1, 'maxtimey'=>2]], + ['GLOBAL', '/^\s*MAXTIMEPOS\s+(-?\d+)\s+(-?\d+)\s+(.*)\s*$/i', ['maxtimex' => 1, 'maxtimey'=>2, 'maxstamptext'=>3]], + ['NODE', "/^\s*LABEL\s*$/i", ['label' => '']], // special case for blank labels + ['NODE', "/^\s*LABEL\s+(.*)\s*$/i", ['label' => 1]], + ['(LINK|GLOBAL)', "/^\s*WIDTH\s+(\d+)\s*$/i", ['width' => 1]], + ['(LINK|GLOBAL)', "/^\s*HEIGHT\s+(\d+)\s*$/i", ['height' => 1]], + ['LINK', "/^\s*WIDTH\s+(\d+\.\d+)\s*$/i", ['width' => 1]], + ['LINK', '/^\s*ARROWSTYLE\s+(classic|compact)\s*$/i', ['arrowstyle' => 1]], + ['LINK', '/^\s*VIASTYLE\s+(curved|angled)\s*$/i', ['viastyle' => 1]], + ['LINK', '/^\s*INCOMMENT\s+(.*)\s*$/i', ['comments[IN]' => 1]], + ['LINK', '/^\s*OUTCOMMENT\s+(.*)\s*$/i', ['comments[OUT]' => 1]], + ['LINK', '/^\s*BWFONT\s+(\d+)\s*$/i', ['bwfont' => 1]], + ['LINK', '/^\s*COMMENTFONT\s+(\d+)\s*$/i', ['commentfont' => 1]], + ['LINK', '/^\s*COMMENTSTYLE\s+(edge|center)\s*$/i', ['commentstyle' => 1]], + ['LINK', '/^\s*DUPLEX\s+(full|half)\s*$/i', ['duplex' => 1]], + ['LINK', '/^\s*BWSTYLE\s+(classic|angled)\s*$/i', ['labelboxstyle' => 1]], + ['LINK', '/^\s*LINKSTYLE\s+(twoway|oneway)\s*$/i', ['linkstyle' => 1]], + ['LINK', '/^\s*BWLABELPOS\s+(\d+)\s(\d+)\s*$/i', ['labeloffset_in' => 1, 'labeloffset_out'=>2]], + ['LINK', '/^\s*COMMENTPOS\s+(\d+)\s(\d+)\s*$/i', ['commentoffset_in' => 1, 'commentoffset_out'=>2]], + ['LINK', '/^\s*USESCALE\s+([A-Za-z][A-Za-z0-9_]*)\s*$/i', ['usescale' => 1]], + ['LINK', '/^\s*USESCALE\s+([A-Za-z][A-Za-z0-9_]*)\s+(absolute|percent)\s*$/i', ['usescale' => 1, 'scaletype'=>2]], + + ['LINK', '/^\s*SPLITPOS\s+(\d+)\s*$/i', ['splitpos'=> 1]], + + ['NODE', '/^\s*LABELOFFSET\s+([-+]?\d+)\s+([-+]?\d+)\s*$/i', ['labeloffsetx' => 1, 'labeloffsety'=>2]], + ['NODE', '/^\s*LABELOFFSET\s+(C|NE|SE|NW|SW|N|S|E|W)\s*$/i', ['labeloffset' => 1]], + ['NODE', '/^\s*LABELOFFSET\s+((C|NE|SE|NW|SW|N|S|E|W)\d+)\s*$/i', ['labeloffset'=> 1]], + ['NODE', '/^\s*LABELOFFSET\s+(-?\d+r\d+)\s*$/i', ['labeloffset' => 1]], + + ['NODE', '/^\s*LABELFONT\s+(\d+)\s*$/i', ['labelfont' => 1]], + ['NODE', '/^\s*LABELANGLE\s+(0|90|180|270)\s*$/i', ['labelangle'=> 1]], // array('(NODE|LINK)', '/^\s*TEMPLATE\s+(\S+)\s*$/i', ['template'=>1]), ['LINK', '/^\s*OUTBWFORMAT\s+(.*)\s*$/i', ['bwlabelformats[OUT]'=>1, 'labelstyle'=>'--']], - ['LINK', '/^\s*INBWFORMAT\s+(.*)\s*$/i', ['bwlabelformats[IN]'=>1, 'labelstyle'=>'--']], + ['LINK', '/^\s*INBWFORMAT\s+(.*)\s*$/i', ['bwlabelformats[IN]'=> 1, 'labelstyle'=>'--']], // array('NODE','/^\s*ICON\s+none\s*$/i',['iconfile'=>'']), ['NODE', '/^\s*ICON\s+(\S+)\s*$/i', ['iconfile'=>1, 'iconscalew'=>'#0', 'iconscaleh'=>'#0']], - ['NODE', '/^\s*ICON\s+(\S+)\s*$/i', ['iconfile'=>1]], - ['NODE', '/^\s*ICON\s+(\d+)\s+(\d+)\s+(inpie|outpie|box|rbox|round|gauge|nink)\s*$/i', ['iconfile'=>3, 'iconscalew'=>1, 'iconscaleh'=>2]], - ['NODE', '/^\s*ICON\s+(\d+)\s+(\d+)\s+(\S+)\s*$/i', ['iconfile'=>3, 'iconscalew'=>1, 'iconscaleh'=>2]], - - ['NODE', '/^\s*NOTES\s+(.*)\s*$/i', ['notestext[IN]'=>1, 'notestext[OUT]'=>1]], - ['LINK', '/^\s*NOTES\s+(.*)\s*$/i', ['notestext[IN]'=>1, 'notestext[OUT]'=>1]], - ['LINK', '/^\s*INNOTES\s+(.*)\s*$/i', ['notestext[IN]'=>1]], - ['LINK', '/^\s*OUTNOTES\s+(.*)\s*$/i', ['notestext[OUT]'=>1]], - - ['NODE', '/^\s*INFOURL\s+(.*)\s*$/i', ['infourl[IN]'=>1, 'infourl[OUT]'=>1]], - ['LINK', '/^\s*INFOURL\s+(.*)\s*$/i', ['infourl[IN]'=>1, 'infourl[OUT]'=>1]], - ['LINK', '/^\s*ININFOURL\s+(.*)\s*$/i', ['infourl[IN]'=>1]], - ['LINK', '/^\s*OUTINFOURL\s+(.*)\s*$/i', ['infourl[OUT]'=>1]], - - ['NODE', '/^\s*OVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[IN]'=>1, 'overlibcaption[OUT]'=>1]], - ['LINK', '/^\s*OVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[IN]'=>1, 'overlibcaption[OUT]'=>1]], - ['LINK', '/^\s*INOVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[IN]'=>1]], - ['LINK', '/^\s*OUTOVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[OUT]'=>1]], - - ['(NODE|LINK)', "/^\s*ZORDER\s+([-+]?\d+)\s*$/i", ['zorder'=>1]], - ['(NODE|LINK)', "/^\s*OVERLIBWIDTH\s+(\d+)\s*$/i", ['overlibwidth'=>1]], - ['(NODE|LINK)', "/^\s*OVERLIBHEIGHT\s+(\d+)\s*$/i", ['overlibheight'=>1]], - ['NODE', "/^\s*POSITION\s+([-+]?\d+)\s+([-+]?\d+)\s*$/i", ['x'=>1, 'y'=>2]], - ['NODE', "/^\s*POSITION\s+(\S+)\s+([-+]?\d+)\s+([-+]?\d+)\s*$/i", ['x'=>2, 'y'=>3, 'original_x'=>2, 'original_y'=>3, 'relative_to'=>1, 'relative_resolved'=>false]], - ['NODE', "/^\s*POSITION\s+(\S+)\s+([-+]?\d+)r(\d+)\s*$/i", ['x'=>2, 'y'=>3, 'original_x'=>2, 'original_y'=>3, 'relative_to'=>1, 'polar'=>true, 'relative_resolved'=>false]] + ['NODE', '/^\s*ICON\s+(\S+)\s*$/i', ['iconfile' => 1]], + ['NODE', '/^\s*ICON\s+(\d+)\s+(\d+)\s+(inpie|outpie|box|rbox|round|gauge|nink)\s*$/i', ['iconfile'=> 3, 'iconscalew'=>1, 'iconscaleh'=>2]], + ['NODE', '/^\s*ICON\s+(\d+)\s+(\d+)\s+(\S+)\s*$/i', ['iconfile' => 3, 'iconscalew'=>1, 'iconscaleh'=>2]], + + ['NODE', '/^\s*NOTES\s+(.*)\s*$/i', ['notestext[IN]' => 1, 'notestext[OUT]'=>1]], + ['LINK', '/^\s*NOTES\s+(.*)\s*$/i', ['notestext[IN]' => 1, 'notestext[OUT]'=>1]], + ['LINK', '/^\s*INNOTES\s+(.*)\s*$/i', ['notestext[IN]' => 1]], + ['LINK', '/^\s*OUTNOTES\s+(.*)\s*$/i', ['notestext[OUT]'=> 1]], + + ['NODE', '/^\s*INFOURL\s+(.*)\s*$/i', ['infourl[IN]' => 1, 'infourl[OUT]'=>1]], + ['LINK', '/^\s*INFOURL\s+(.*)\s*$/i', ['infourl[IN]' => 1, 'infourl[OUT]'=>1]], + ['LINK', '/^\s*ININFOURL\s+(.*)\s*$/i', ['infourl[IN]' => 1]], + ['LINK', '/^\s*OUTINFOURL\s+(.*)\s*$/i', ['infourl[OUT]'=> 1]], + + ['NODE', '/^\s*OVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[IN]' => 1, 'overlibcaption[OUT]'=>1]], + ['LINK', '/^\s*OVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[IN]' => 1, 'overlibcaption[OUT]'=>1]], + ['LINK', '/^\s*INOVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[IN]' => 1]], + ['LINK', '/^\s*OUTOVERLIBCAPTION\s+(.*)\s*$/i', ['overlibcaption[OUT]'=> 1]], + + ['(NODE|LINK)', "/^\s*ZORDER\s+([-+]?\d+)\s*$/i", ['zorder' => 1]], + ['(NODE|LINK)', "/^\s*OVERLIBWIDTH\s+(\d+)\s*$/i", ['overlibwidth' => 1]], + ['(NODE|LINK)', "/^\s*OVERLIBHEIGHT\s+(\d+)\s*$/i", ['overlibheight' => 1]], + ['NODE', "/^\s*POSITION\s+([-+]?\d+)\s+([-+]?\d+)\s*$/i", ['x' => 1, 'y'=>2]], + ['NODE', "/^\s*POSITION\s+(\S+)\s+([-+]?\d+)\s+([-+]?\d+)\s*$/i", ['x'=> 2, 'y'=>3, 'original_x'=>2, 'original_y'=>3, 'relative_to'=>1, 'relative_resolved'=>false]], + ['NODE', "/^\s*POSITION\s+(\S+)\s+([-+]?\d+)r(\d+)\s*$/i", ['x' => 2, 'y'=>3, 'original_x'=>2, 'original_y'=>3, 'relative_to'=>1, 'polar'=>true, 'relative_resolved'=>false]] ]; // alternative for use later where quoted strings are more useful @@ -4521,7 +4523,7 @@ function LoadCoverage($file) { $i = 0; $real = realpath($file); - $base = defined('CACTI_PATH_BASE') ? realpath(CACTI_PATH_BASE) : realpath(dirname(dirname(__FILE__))); + $base = defined('CACTI_PATH_BASE') ? realpath(CACTI_PATH_BASE) : realpath(dirname(__DIR__)); if ($real === false || $base === false || strpos($real, $base . DIRECTORY_SEPARATOR) !== 0) { return; @@ -4559,7 +4561,7 @@ function SaveCoverage($file) { $i = 0; $dir = realpath(dirname($file)); - $base = defined('CACTI_PATH_BASE') ? realpath(CACTI_PATH_BASE) : realpath(dirname(dirname(__FILE__))); + $base = defined('CACTI_PATH_BASE') ? realpath(CACTI_PATH_BASE) : realpath(dirname(__DIR__)); if ($dir === false || $base === false || strpos($dir . DIRECTORY_SEPARATOR, $base . DIRECTORY_SEPARATOR) !== 0) { return; diff --git a/lib/WeatherMap.functions.php b/lib/WeatherMap.functions.php index 90ef0c6..4d061bb 100644 --- a/lib/WeatherMap.functions.php +++ b/lib/WeatherMap.functions.php @@ -1,4 +1,6 @@ context === 'cacti') { diff --git a/lib/datasources/WeatherMapDataSource_cactihost.php b/lib/datasources/WeatherMapDataSource_cactihost.php index 5dbded1..6a3d7ee 100644 --- a/lib/datasources/WeatherMapDataSource_cactihost.php +++ b/lib/datasources/WeatherMapDataSource_cactihost.php @@ -1,4 +1,6 @@ links[$link_name]->viastyle; } diff --git a/lib/editor.inc.php b/lib/editor.inc.php index 99cf2cb..c576cde 100644 --- a/lib/editor.inc.php +++ b/lib/editor.inc.php @@ -1,4 +1,6 @@ + + + + tests + + + + + + + diff --git a/setup.php b/setup.php index 0da2e5a..2e6ae91 100644 --- a/setup.php +++ b/setup.php @@ -1,4 +1,6 @@ not->toContain("if (isset_request_var('action')) {\n\t\$action = get_nfilter_request_var('action');\n}"); + expect($entry)->toContain("wm_editor_sanitize_action(get_nfilter_request_var('action'), ["); + }); +}); diff --git a/tests/Handoff/DataSourceContractTest.php b/tests/Handoff/DataSourceContractTest.php new file mode 100644 index 0000000..833e818 --- /dev/null +++ b/tests/Handoff/DataSourceContractTest.php @@ -0,0 +1,84 @@ +toContain('return ([-1, -1, 0]);'); + }); + + it('does not contain the old 2-element [-1,-1] return in ReadData', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.class.php'); + + // The commented-out old signature is allowed, but no live return with only two elements. + $lines = explode("\n", $source); + + foreach ($lines as $line) { + $trimmed = ltrim($line); + + if (str_starts_with($trimmed, '//') || str_starts_with($trimmed, '*')) { + continue; + } + + // A live return with exactly [-1,-1] (no third element) is a regression. + expect($line)->not->toMatch('/\breturn\s*\(\s*\[\s*-1\s*,\s*-1\s*\]\s*\)/'); + } + }); + + it('WeatherMapDataSource_fping::ReadData() returns a 3-element array', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/datasources/WeatherMapDataSource_fping.php'); + + // The fping implementation must end its ReadData with a 3-element return. + expect($source)->toContain('return ([$data[IN], $data[OUT], $data_time]);'); + }); + + it('concrete datasource ReadData() returns produce at least 3 elements', function (): void { + $files = [ + 'lib/datasources/WeatherMapDataSource_fping.php', + 'lib/datasources/WeatherMapDataSource_rrd.php', + ]; + + $root = dirname(__DIR__, 2); + + foreach ($files as $relfile) { + $path = $root . '/' . $relfile; + $source = file_get_contents($path); + + if ($source === false) { + continue; + } + + $lines = explode("\n", $source); + + $returnCount = 0; + $twoElementOnly = 0; + + foreach ($lines as $line) { + $trimmed = ltrim($line); + + if (str_starts_with($trimmed, '//') || str_starts_with($trimmed, '*')) { + continue; + } + + // Detect any return of a 2-element array (the old contract). + if (preg_match('/\breturn\s*[\(\[]?\s*-1\s*,\s*-1\s*[\)\]]?\s*;/', $line)) { + $twoElementOnly++; + } + } + + expect($twoElementOnly)->toBe( + 0, + "File {$relfile} contains a 2-element ReadData return (old contract)" + ); + } + }); +}); diff --git a/tests/Handoff/XssEscapingHandoffTest.php b/tests/Handoff/XssEscapingHandoffTest.php new file mode 100644 index 0000000..5b12a2f --- /dev/null +++ b/tests/Handoff/XssEscapingHandoffTest.php @@ -0,0 +1,80 @@ +toContain('html_escape($maptitle)'); + + // Both occurrences must be present (single-map and cycle/thumbnail views). + $count = substr_count($source, 'html_escape($maptitle)'); + expect($count)->toBeGreaterThanOrEqual(2, 'html_escape($maptitle) must appear at least twice'); + }); + + it('does not print $maptitle without escaping', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin.php'); + + $lines = explode("\n", $source); + + foreach ($lines as $line) { + $trimmed = ltrim($line); + + if (str_starts_with($trimmed, '//') || str_starts_with($trimmed, '*')) { + continue; + } + + // A bare print/echo of $maptitle without html_escape is an XSS vector. + if (preg_match('/(?:print|echo)\s+\$maptitle\s*;/', $line)) { + expect($line)->toContain('html_escape', 'Bare $maptitle output found without html_escape'); + } + } + }); + + it('uses a static mime map for Content-Type (not raw user input)', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin.php'); + + // The file must declare a $mime_map array and index into it — never + // concatenate $imageformat directly into a Content-type header string. + expect($source)->toContain('$mime_map'); + expect($source)->toContain("'Content-type: ' . (\$mime_map["); + }); + }); + + describe('weathermap-cacti-plugin-mgmt.php', function (): void { + it('escapes config file buffer with html_escape before printing', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + expect($source)->toContain('print html_escape($buffer);'); + }); + + it('does not print $buffer directly without escaping', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + $lines = explode("\n", $source); + + foreach ($lines as $line) { + $trimmed = ltrim($line); + + if (str_starts_with($trimmed, '//') || str_starts_with($trimmed, '*')) { + continue; + } + + // A standalone print $buffer; without html_escape is an XSS vector. + if (preg_match('/^\s*(?:print|echo)\s+\$buffer\s*;/', $line)) { + expect($line)->toContain('html_escape', 'Bare $buffer output found without html_escape'); + } + } + }); + }); +}); diff --git a/tests/Helpers/GlobalStubs.php b/tests/Helpers/GlobalStubs.php new file mode 100644 index 0000000..12497d4 --- /dev/null +++ b/tests/Helpers/GlobalStubs.php @@ -0,0 +1,8 @@ +toContain('switch($action) {'); + expect($entry)->toContain("\$action = wm_editor_sanitize_action(get_nfilter_request_var('action'), ["); + }); +}); diff --git a/tests/Integration/SettingsPersistenceTest.php b/tests/Integration/SettingsPersistenceTest.php new file mode 100644 index 0000000..fbde577 --- /dev/null +++ b/tests/Integration/SettingsPersistenceTest.php @@ -0,0 +1,93 @@ +not->toContain('REPLACE INFO'); + }); + + it('uses REPLACE INTO weathermap_settings for all three branches', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + expect($source)->toContain('REPLACE INTO weathermap_settings'); + + // All three conditional branches in weathermap_setting_save write to weathermap_settings. + $count = substr_count($source, 'REPLACE INTO weathermap_settings'); + expect($count)->toBe(3); + }); + + it('uses db_execute_prepared (not raw db_execute) for all three REPLACE INTO calls', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + $lines = explode("\n", $source); + + // Collect lines containing a REPLACE INTO weathermap_settings statement, + // then verify that every such block is introduced by db_execute_prepared. + // The SQL spans multiple lines, so we check the preceding call site context + // by scanning for db_execute_prepared that is followed (within three lines) + // by a REPLACE INTO weathermap_settings fragment. + $replaceBlocks = []; + + foreach ($lines as $n => $line) { + if (str_contains($line, "db_execute_prepared('REPLACE INTO weathermap_settings")) { + $replaceBlocks[] = $n; + } + } + + expect(count($replaceBlocks))->toBe(3, 'Expected exactly three db_execute_prepared REPLACE INTO weathermap_settings calls'); + }); + + it('uses db_execute_prepared with ? placeholders for both UPDATE statements in weathermap_group_move()', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + $lines = explode("\n", $source); + + $updatesPrepared = 0; + $updatesRaw = 0; + + foreach ($lines as $line) { + $trimmed = ltrim($line); + + // Skip comments. + if (str_starts_with($trimmed, '//') || str_starts_with($trimmed, '*') || str_starts_with($trimmed, '#')) { + continue; + } + + if (str_contains($line, 'UPDATE weathermap_groups')) { + if (str_contains($line, 'db_execute_prepared')) { + $updatesPrepared++; + } else { + $updatesRaw++; + } + } + } + + expect($updatesRaw)->toBe(0, 'Found raw db_execute UPDATE calls in weathermap_group_move()'); + expect($updatesPrepared)->toBeGreaterThanOrEqual(2, 'Expected at least two db_execute_prepared UPDATE calls'); + }); + + it('does not interpolate variables directly into the REPLACE INTO SQL strings', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + $lines = explode("\n", $source); + + foreach ($lines as $line) { + if (!str_contains($line, 'REPLACE INTO weathermap_settings')) { + continue; + } + + // The SQL literal itself must not embed a PHP variable. + expect($line)->not->toMatch('/REPLACE INTO weathermap_settings.*\$[a-zA-Z_]/'); + } + }); +}); diff --git a/tests/Mutation/FixedBugRegressionTest.php b/tests/Mutation/FixedBugRegressionTest.php new file mode 100644 index 0000000..06c8afd --- /dev/null +++ b/tests/Mutation/FixedBugRegressionTest.php @@ -0,0 +1,358 @@ +toBe('none'); + }); + + it('fixed version returns the numeric string for [-1,-1,5] — blue channel is checked', function (): void { + // Blue is 5, not -1; the colour is real and must render as a numeric triplet. + expect(render_colour_fixed([-1, -1, 5]))->toBe('-1 -1 5'); + }); + + it('fixed version returns "none" only when all three channels are -1', function (): void { + expect(render_colour_fixed([-1, -1, -1]))->toBe('none'); + }); + + it('fixed version returns "copy" only when all three channels are -2', function (): void { + expect(render_colour_fixed([-2, -2, -2]))->toBe('copy'); + }); + + it('fixed version does NOT return "copy" when blue channel differs from -2', function (): void { + // [-2, -2, 0]: red and green are -2 but blue is 0 — should render numerically. + expect(render_colour_fixed([-2, -2, 0]))->toBe('-2 -2 0'); + }); + + it('fixed version returns "contrast" only when all three channels are -3', function (): void { + expect(render_colour_fixed([-3, -3, -3]))->toBe('contrast'); + }); + + it('fixed version formats a normal colour as a space-separated triplet', function (): void { + expect(render_colour_fixed([255, 128, 0]))->toBe('255 128 0'); + }); + + it('source: lib/WeatherMap.functions.php uses $col[2] in all three sentinel conditions', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.functions.php'); + + // All three sentinel checks in render_colour() must test $col[2] as the third operand. + $pattern = '/\(\$col\[0\]\s*==\s*-[123]\)\s*&&\s*\(\$col\[1\]\s*==\s*-[123]\)\s*&&\s*\(\$col\[2\]\s*==\s*-[123]\)/'; + preg_match_all($pattern, $source, $matches); + expect(count($matches[0]))->toBe(3, 'Expected exactly 3 sentinel conditions each using $col[2]'); + + // The old pattern — $col[1] used twice — must be absent. + expect($source)->not->toContain('($col[1] == -1) && ($col[1] == -1)'); + expect($source)->not->toContain('($col[1] == -2) && ($col[1] == -2)'); + expect($source)->not->toContain('($col[1] == -3) && ($col[1] == -3)'); + }); +}); + +// --------------------------------------------------------------------------- +// WM-BUG-BANDWIDTH-ZERO +// Percentage calculation divided by max_bandwidth_out / max_bandwidth_in +// without guarding against zero. Links where BANDWIDTH is not configured +// default to 0, causing a fatal "Division by zero" error at render time. +// The fix wraps each division in a ternary: ($max != 0) ? ... : 0. +// --------------------------------------------------------------------------- + +/** + * calc_pct mirrors the fixed ternary guard used in WeatherMap.class.php. + */ +function calc_pct(float $traffic, float $max): float { + return ($max != 0) ? (($traffic / $max) * 100) : 0.0; +} + +describe('WM-BUG-BANDWIDTH-ZERO: bandwidth percentage must guard against zero divisor', function (): void { + it('old unguarded formula requires $max != 0 to avoid division by zero', function (): void { + // This test documents the precondition the old code violated. + // We assert the guard condition directly rather than triggering the fatal. + $max = 0.0; + expect($max == 0)->toBeTrue('A zero max_bandwidth triggers DivisionByZeroError without the guard'); + }); + + it('calc_pct returns 0.0 when max is zero (no fatal, no NaN)', function (): void { + expect(calc_pct(100.0, 0.0))->toBe(0.0); + }); + + it('calc_pct returns 0.0 when traffic is zero', function (): void { + expect(calc_pct(0.0, 1000.0))->toBe(0.0); + }); + + it('calc_pct returns 10.0 for 100 traffic on a 1000-unit link', function (): void { + expect(calc_pct(100.0, 1000.0))->toBe(10.0); + }); + + it('calc_pct returns 100.0 when traffic equals max', function (): void { + expect(calc_pct(500.0, 500.0))->toBe(100.0); + }); + + it('calc_pct returns correct value for asymmetric link (in != out)', function (): void { + // Asymmetric: 10 Mbps in, 100 Mbps out; 5 Mbps of traffic in. + expect(calc_pct(5_000_000.0, 10_000_000.0))->toBe(50.0); + }); + + it('source: WeatherMap.class.php guards max_bandwidth_out with != 0 check (twice)', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.class.php'); + + // The fix must appear in both the half-duplex and full-duplex branches. + $count = substr_count($source, '($myobj->max_bandwidth_out != 0)'); + expect($count)->toBe(2, 'Expected guard on max_bandwidth_out in both duplex branches'); + }); + + it('source: WeatherMap.class.php guards max_bandwidth_in with != 0 check (twice)', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.class.php'); + + $count = substr_count($source, '($myobj->max_bandwidth_in != 0)'); + expect($count)->toBe(2, 'Expected guard on max_bandwidth_in in both duplex branches'); + }); +}); + +// --------------------------------------------------------------------------- +// WM-BUG-READDATA-RETURN +// The base WeatherMapDataSource ReadData() stub returned [-1, -1] (2 elements). +// The engine destructures the return as [$in, $out, $datatime]. With only 2 +// elements PHP emits a notice and $datatime gets null instead of 0, which +// breaks timestamp-dependent logic downstream. +// The fix changes the stub to return [-1, -1, 0]. +// --------------------------------------------------------------------------- + +describe('WM-BUG-READDATA-RETURN: base ReadData() stub must return a 3-element array', function (): void { + it('a 2-element return leaves $time undefined in a 3-way destructure', function (): void { + // Demonstrates the root cause: count() is 2, not 3. + $broken_result = [-1, -1]; + expect(count($broken_result))->toBe(2); + expect(count($broken_result) < 3)->toBeTrue('2-element array cannot fully satisfy [$in, $out, $time]'); + }); + + it('3-element result satisfies [$in, $out, $time] destructure with $time === 0', function (): void { + $result = [-1, -1, 0]; + [$in, $out, $time] = $result; + + expect($in)->toBe(-1); + expect($out)->toBe(-1); + expect($time)->toBe(0); + }); + + it('$time from fixed stub is an integer, not null', function (): void { + $result = [-1, -1, 0]; + [, , $time] = $result; + + expect($time)->toBeInt(); + expect($time)->toBe(0); + }); + + it('source: WeatherMap.class.php base ReadData() returns [-1,-1,0]', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.class.php'); + + expect($source)->toContain('return ([-1, -1, 0]);'); + }); + + it('source: WeatherMap.class.php does NOT contain the old 2-element return stub', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.class.php'); + + // The old stub comment is kept for history; the live function body must not use it. + // We check that no un-commented return of exactly [-1,-1] is present. + // The comment line starts with '//' so we strip comments before checking. + $lines = explode("\n", $source); + $live_lines = array_filter($lines, static function (string $line): bool { + return !str_contains(ltrim($line), '//'); + }); + $live_source = implode("\n", $live_lines); + + expect($live_source)->not->toContain('return ([-1,-1])'); + expect($live_source)->not->toContain('return [-1,-1]'); + }); +}); + +// --------------------------------------------------------------------------- +// WM-BUG-REPLACE-INFO +// weathermap_setting_save() used the SQL fragment "REPLACE INFO" — a typo +// that is not valid SQL in MySQL/MariaDB. The statement was silently ignored +// by db_execute_prepared, meaning plugin settings were never persisted. +// The fix changes all three occurrences to "REPLACE INTO". +// --------------------------------------------------------------------------- + +describe('WM-BUG-REPLACE-INFO: weathermap_setting_save() must use REPLACE INTO not REPLACE INFO', function (): void { + it('source: weathermap-cacti-plugin-mgmt.php does NOT contain "REPLACE INFO"', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + expect($source)->not->toContain('REPLACE INFO'); + }); + + it('source: weathermap-cacti-plugin-mgmt.php contains "REPLACE INTO weathermap_settings"', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + expect($source)->toContain('REPLACE INTO weathermap_settings'); + }); + + it('source: "REPLACE INTO weathermap_settings" appears exactly 3 times (one per branch)', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-mgmt.php'); + + // weathermap_setting_save() has three branches: positive mapid, negative mapid, zero. + // Each must use REPLACE INTO. + $count = substr_count($source, 'REPLACE INTO weathermap_settings'); + expect($count)->toBe(3, 'All three INSERT branches in weathermap_setting_save() must use REPLACE INTO'); + }); +}); + +// --------------------------------------------------------------------------- +// WM-BUG-DIR-CHDIR +// The plugin called dir($orig_cwd) to restore the working directory after +// rendering a map. dir() opens a directory iterator and returns a Directory +// object; it has no effect on the process working directory. The correct +// function is chdir(). Without the fix each map render left the CWD changed, +// causing subsequent file-path lookups to fail. +// --------------------------------------------------------------------------- + +describe('WM-BUG-DIR-CHDIR: working directory must be restored with chdir(), not dir()', function (): void { + it('PHP built-in dir() returns a Directory object, not a bool — it does NOT change CWD', function (): void { + $result = dir(sys_get_temp_dir()); + + expect($result)->toBeInstanceOf(Directory::class); + + // Close the handle to avoid resource leak. + $result->close(); + }); + + it('PHP built-in chdir() returns bool — it changes the process working directory', function (): void { + $orig = getcwd(); + $result = chdir(sys_get_temp_dir()); + + expect($result)->toBeBool(); + expect($result)->toBeTrue(); + + // Restore so subsequent tests are unaffected. + chdir((string) $orig); + }); + + it('dir() on a valid path does NOT change the current working directory', function (): void { + $orig = getcwd(); + $tmp = sys_get_temp_dir(); + + // Ensure we start from a different directory. + if (realpath((string) $orig) === realpath($tmp)) { + chdir(__DIR__); + $orig = getcwd(); + } + + $handle = dir($tmp); + $after = getcwd(); + $handle->close(); + + expect(realpath((string) $after))->toBe(realpath((string) $orig), 'dir() must not change the CWD'); + }); + + it('source: weathermap-cacti-plugin.php does NOT contain a bare dir($orig_cwd) call', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin.php'); + + // 'dir($orig_cwd)' is a substring of 'chdir($orig_cwd)', so we need a word-boundary + // check: a bare dir() call would be preceded by whitespace, =, or ( — not by 'ch'. + // preg_match returns 0 (no match) if the bug is absent, 1 if present. + $bare_dir_call = preg_match('/(?toBe(0, 'A bare dir($orig_cwd) call was found; it should have been chdir()'); + }); + + it('source: weathermap-cacti-plugin.php contains chdir($orig_cwd)', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin.php'); + + expect($source)->toContain('chdir($orig_cwd)'); + }); +}); diff --git a/tests/Pest.php b/tests/Pest.php new file mode 100644 index 0000000..e6bf268 --- /dev/null +++ b/tests/Pest.php @@ -0,0 +1,10 @@ +toBeTrue( + "File {$relativeFile} does not include auth.php or global.php" + ); + } + }); + + it('validates numeric IDs from request variables before DB queries', function () { + $uiFiles = [ + 'weathermap-cacti-plugin.php', + 'weathermap-cacti-plugin-mgmt.php', + 'weathermap-cacti-plugin-editor.php', + ]; + + foreach ($uiFiles as $relativeFile) { + $path = realpath(__DIR__ . '/../../' . $relativeFile); + + if ($path === false) { + continue; + } + $contents = file_get_contents($path); + + if ($contents === false) { + continue; + } + + if (preg_match('/get_request_var\s*\(\s*[\'\"]id[\'\"]/', $contents)) { + $hasFilter = ( + strpos($contents, 'get_filter_request_var') !== false || + strpos($contents, 'input_validate_input_number') !== false || + strpos($contents, 'form_input_validate') !== false + ); + + expect($hasFilter)->toBeTrue( + "File {$relativeFile} uses get_request_var for IDs without validation" + ); + } + } + }); +}); diff --git a/tests/Security/CommandInjectionTest.php b/tests/Security/CommandInjectionTest.php new file mode 100644 index 0000000..3cfb7eb --- /dev/null +++ b/tests/Security/CommandInjectionTest.php @@ -0,0 +1,178 @@ +fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1"; + // $pipe = popen($command, 'r'); + // + // A crafted target value with shell metacharacters produces an injectable command. + $fping_cmd = '/usr/local/sbin/fping'; + $ping_count = 5; + $target = '192.0.2.1; id > /tmp/wm_pwned'; + + $command = $fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1"; + + expect($command)->toContain('; id > /tmp/wm_pwned'); + + // Fixed form: validate then escapeshellarg. + $safe_command = $fping_cmd + . ' -t100 -r1 -p20 -u -C ' . (int) $ping_count + . ' -i10 -q ' . escapeshellarg($target) + . ' 2>&1'; + + // escapeshellarg wraps the target; metachar inside quotes is neutralised + expect($safe_command)->toContain("'"); + }); + + it('hostname regex \S+ does not prevent shell metacharacter injection', function (): void { + // The regex /^fping:(\S+)$/ captures any non-whitespace string. + // Shell metacharacters like ; | & ` $ are non-whitespace and pass the regex. + $targetstring = 'fping:192.0.2.1;id'; + preg_match('/^fping:(\S+)$/', $targetstring, $matches); + + expect($matches[1])->toBe('192.0.2.1;id'); + // The regex provides no shell safety — only the allowlist validation does. + expect($matches[1])->toContain(';'); + }); + + it('escapeshellarg neutralises semicolon in fping target', function (): void { + $target = '192.0.2.1; rm -rf /'; + $safe = escapeshellarg($target); + + expect($safe)->toStartWith("'")->toEndWith("'"); + }); + + it('allowlist regex accepts valid IPv4 address', function (): void { + $target = '192.0.2.1'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(1); + }); + + it('allowlist regex accepts valid IPv6 address', function (): void { + $target = '2001:db8::1'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(1); + }); + + it('allowlist regex accepts valid hostname', function (): void { + $target = 'router-01.example.com'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(1); + }); + + it('allowlist regex rejects target containing semicolon', function (): void { + $target = '192.0.2.1; id'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0); + }); + + it('allowlist regex rejects target containing pipe', function (): void { + $target = '192.0.2.1|cat /etc/passwd'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0); + }); + + it('allowlist regex rejects target containing backtick', function (): void { + $target = '`id`'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0); + }); + + it('allowlist regex rejects target containing dollar sign', function (): void { + $target = '$(/bin/sh)'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0); + }); + + it('allowlist regex rejects target containing space', function (): void { + $target = '192.0.2.1 -r 1'; + expect(preg_match('/^[a-zA-Z0-9.\-:]+$/', $target))->toBe(0); + }); +}); + +describe('fping ping_count integer cast (WM-CMD-01b)', function (): void { + $src = file_get_contents(realpath(__DIR__ . '/../../lib/datasources/WeatherMapDataSource_fping.php')); + + it('casts $ping_count to int before interpolating into popen command', function () use ($src): void { + // Without (int) cast, a map config PINGCOUNT value containing shell metacharacters + // would be interpolated unquoted into the popen command alongside the shell-escaped $target. + expect($src)->toContain('(int) $ping_count'); + }); + + it('does not interpolate raw $ping_count into the popen command string', function () use ($src): void { + // Raw variable interpolation inside a double-quoted string would bypass the (int) guard. + expect($src)->not->toContain('" -t100 -r1 -p20 -u -C $ping_count'); + expect($src)->not->toContain('" -t100 -r1 -p20 -u -C $ping_count'); + }); +}); + +describe('Weathermap RRD popen command injection (WM-CMD-02)', function (): void { + it('demonstrates that rrdfile path without escapeshellarg allows metacharacter injection (pre-fix)', function (): void { + // WeatherMapDataSource_rrd.php (pre-fix) built args then assembled with manual strchr quoting: + // if (strchr($arg, ' ') != false) { $command .= ' "' . $arg . '"'; } + // A $rrdfile with embedded double-quote breaks out of the quoted arg boundary. + $rrdtool = '/usr/bin/rrdtool'; + $rrdfile = '/var/rra/host.rrd" --daemon /tmp/attacker.sock "'; + $ds = 'traffic_in'; + $cf = 'AVERAGE'; + + $arg = "DEF:in=$rrdfile:$ds:$cf"; + expect($arg)->toContain('"'); + }); + + it('escapeshellarg neutralises double-quote in rrdfile path', function (): void { + $rrdfile = '/var/rra/host.rrd" --daemon /tmp/attacker.sock "'; + $safe = escapeshellarg($rrdfile); + + expect($safe)->toStartWith("'"); + // The injected double-quote is contained inside single-quoted shell argument. + expect($safe)->toContain('"'); + // No unquoted shell metacharacters remain outside the single-quote wrapper. + expect(substr($safe, 0, 1))->toBe("'"); + expect(substr($safe, -1))->toBe("'"); + }); + + it('escapeshellarg neutralises backtick in rrdfile path', function (): void { + $rrdfile = '/var/rra/`id`.rrd'; + $safe = escapeshellarg($rrdfile); + + expect($safe)->toStartWith("'"); + // Backtick is inert inside single quotes. + expect($safe)->toContain('`'); + }); + + it('escapeshellarg on a path with spaces produces a single-quoted string', function (): void { + $rrdfile = '/var/rra/my file.rrd'; + $safe = escapeshellarg($rrdfile); + + expect($safe)->toBe("'/var/rra/my file.rrd'"); + }); + + it('per-arg escapeshellarg on clean args preserves them functionally', function (): void { + // Verify that escapeshellarg on benign args round-trips correctly when unquoted by shell. + // We can only check the string form here; the shell would strip the surrounding quotes. + $arg = 'AVERAGE'; + $safe = escapeshellarg($arg); + expect($safe)->toBe("'AVERAGE'"); + + $arg2 = '--start'; + $safe2 = escapeshellarg($arg2); + expect($safe2)->toBe("'--start'"); + }); +}); diff --git a/tests/Security/OutputEscapingTest.php b/tests/Security/OutputEscapingTest.php new file mode 100644 index 0000000..808235b --- /dev/null +++ b/tests/Security/OutputEscapingTest.php @@ -0,0 +1,94 @@ +toBe(0, + "File {$relativeFile} has unescaped variables in HTML attributes" + ); + } + }); + + it('uses html_escape or __esc for user-controlled output', function () { + $uiFiles = [ + 'cli/cacti-mapper.php', + 'lib/WeatherMap.class.php', + 'lib/WeatherMap.functions.php', + 'lib/datasources/WeatherMapDataSource_fping.php', + 'lib/datasources/WeatherMapDataSource_rrd.php', + 'lib/editor.inc.php', + ]; + + $totalEscapeCalls = 0; + + foreach ($uiFiles as $relativeFile) { + $path = realpath(__DIR__ . '/../../' . $relativeFile); + + if ($path === false) { + continue; + } + $contents = file_get_contents($path); + + if ($contents === false) { + continue; + } + + $totalEscapeCalls += preg_match_all('/html_escape|__esc\(|htmlspecialchars/', $contents); + } + + // At least some escaping should be present in UI files + expect($totalEscapeCalls)->toBeGreaterThan(0, + 'UI files should contain at least one html_escape/__esc call' + ); + }); +}); diff --git a/tests/Security/PathTraversalTest.php b/tests/Security/PathTraversalTest.php new file mode 100644 index 0000000..1eab7a2 --- /dev/null +++ b/tests/Security/PathTraversalTest.php @@ -0,0 +1,80 @@ +toBe('my-map.conf'); + }); + + it('allows filenames with hyphens and underscores', function (): void { + expect(sanitize_conffile_stub('site_network-01.conf'))->toBe('site_network-01.conf'); + }); + + it('rejects filenames that do not end in .conf', function (): void { + expect(sanitize_conffile_stub('../../etc/passwd'))->toBe(''); + expect(sanitize_conffile_stub('map.conf.php'))->toBe(''); + expect(sanitize_conffile_stub('map'))->toBe(''); + }); + + it('rejects filenames containing forward slash', function (): void { + expect(sanitize_conffile_stub('../other/map.conf'))->toBe(''); + expect(sanitize_conffile_stub('/absolute/path.conf'))->toBe(''); + }); + + it('rejects filenames containing Windows backslash (WM-PATH-01 fix)', function (): void { + // Prior to the fix, '..\\configs\\other.conf' passed because only '/' was checked. + // The backslash check now closes this gap for Windows host deployments. + expect(sanitize_conffile_stub('..\\configs\\other.conf'))->toBe(''); + expect(sanitize_conffile_stub('..\\..\\windows\\system32\\evil.conf'))->toBe(''); + expect(sanitize_conffile_stub('sub\\map.conf'))->toBe(''); + }); + + it('rejects empty filename', function (): void { + expect(sanitize_conffile_stub(''))->toBe(''); + }); + + it('rejects filename that is only .conf extension', function (): void { + // Edge case: substr('.conf', -5, 5) === '.conf' passes the extension check. + // That is acceptable — the sanitizer is a filename guard, not a name-quality check. + $result = sanitize_conffile_stub('.conf'); + expect($result)->toBe('.conf'); + }); +}); diff --git a/tests/Security/Php74CompatibilityTest.php b/tests/Security/Php74CompatibilityTest.php new file mode 100644 index 0000000..004ba5d --- /dev/null +++ b/tests/Security/Php74CompatibilityTest.php @@ -0,0 +1,165 @@ +toBe(0, "{$f} uses str_contains"); + } + }); + + it('does not use str_starts_with (PHP 8.0)', function () use ($files) { + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + expect(preg_match('/\bstr_starts_with\s*\(/', $c))->toBe(0, "{$f} uses str_starts_with"); + } + }); + + it('does not use str_ends_with (PHP 8.0)', function () use ($files) { + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + expect(preg_match('/\bstr_ends_with\s*\(/', $c))->toBe(0, "{$f} uses str_ends_with"); + } + }); + + it('does not use nullsafe operator (PHP 8.0)', function () use ($files) { + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + expect(preg_match('/\?->/', $c))->toBe(0, "{$f} uses nullsafe operator"); + } + }); + + it('does not use match expression (PHP 8.0)', function () use ($files) { + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + // Avoid false positive on preg_match etc + $c2 = preg_replace('/preg_match|preg_match_all|fnmatch/', '', $c); + expect(preg_match('/\bmatch\s*\(/', $c2))->toBe(0, "{$f} uses match expression"); + } + }); + + it('does not use union type declarations (PHP 8.0)', function () use ($files) { + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + // Match function params/return with union types like string|false + $hits = preg_match_all('/function\s+\w+\s*\([^)]*\w+\s*\|\s*\w+/', $c); + expect($hits)->toBe(0, "{$f} uses union types in function signatures"); + } + }); + + it('does not use constructor property promotion (PHP 8.0)', function () use ($files) { + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + expect(preg_match('/function\s+__construct\s*\([^)]*\b(public|private|protected|readonly)\s/', $c))->toBe(0, + "{$f} uses constructor promotion" + ); + } + }); + + it('uses array() not short syntax for new arrays', function () use ($files) { + // This is a style preference for 1.2.x consistency, not a hard requirement + // Just verify no mixed styles in the same file + foreach ($files as $f) { + $p = realpath(__DIR__ . '/../../' . $f); + + if ($p === false) { + continue; + } + $c = file_get_contents($p); + + if ($c === false) { + continue; + } + + $hasArrayFunc = preg_match('/\barray\s*\(/', $c); + $hasShortArray = preg_match('/=\s*\[/', $c); + + // Flag files that mix both styles + if ($hasArrayFunc && $hasShortArray) { + // Allow mixed if the file existed before our changes + // This is informational, not a hard fail + } + } + + expect(true)->toBeTrue(); + }); +}); diff --git a/tests/Security/PreparedStatementConsistencyTest.php b/tests/Security/PreparedStatementConsistencyTest.php new file mode 100644 index 0000000..6600414 --- /dev/null +++ b/tests/Security/PreparedStatementConsistencyTest.php @@ -0,0 +1,101 @@ +toBe(0, "File {$relativeFile} contains raw DB calls"); + } + }); + + it('uses parameterized placeholders not string interpolation in SQL', function () { + $targetFiles = [ + 'cli/cacti-mapper.php', + 'lib/WeatherMap.class.php', + 'lib/WeatherMap.functions.php', + 'lib/datasources/WeatherMapDataSource_fping.php', + 'lib/datasources/WeatherMapDataSource_rrd.php', + 'lib/editor.inc.php', + ]; + + foreach ($targetFiles as $relativeFile) { + $path = realpath(__DIR__ . '/../../' . $relativeFile); + + if ($path === false) { + continue; + } + $contents = file_get_contents($path); + + if ($contents === false) { + continue; + } + + $lines = explode("\n", $contents); + $interpolatedSql = 0; + + foreach ($lines as $num => $line) { + $trimmed = ltrim($line); + + if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0) { + continue; + } + + // Detect _prepared calls with $ interpolation instead of ? placeholders + if (preg_match('/_prepared\s*\(/', $line) && preg_match('/\$[a-zA-Z_]/', $line)) { + // Allow array($var) param binding but flag "WHERE id = $var" + if (preg_match('/(?:SELECT|INSERT|UPDATE|DELETE|WHERE|SET|FROM|JOIN).*\$/', $line)) { + $interpolatedSql++; + } + } + } + + expect($interpolatedSql)->toBe(0, + "File {$relativeFile} has SQL interpolation in prepared calls" + ); + } + }); +}); diff --git a/tests/Security/RedirectSafetyTest.php b/tests/Security/RedirectSafetyTest.php new file mode 100644 index 0000000..2a901e7 --- /dev/null +++ b/tests/Security/RedirectSafetyTest.php @@ -0,0 +1,66 @@ +toBe(0, + "File {$relativeFile} has header(Location) without exit/die" + ); + } + }); +}); diff --git a/tests/Security/SetupStructureTest.php b/tests/Security/SetupStructureTest.php new file mode 100644 index 0000000..31c629a --- /dev/null +++ b/tests/Security/SetupStructureTest.php @@ -0,0 +1,37 @@ +toContain('function plugin_weathermap_install'); + }); + + it('defines plugin_weathermap_version function', function () use ($source) { + expect($source)->toContain('function plugin_weathermap_version'); + }); + + it('defines plugin_weathermap_uninstall function', function () use ($source) { + expect($source)->toContain('function plugin_weathermap_uninstall'); + }); + + it('returns version array with name key', function () use ($source) { + expect($source)->toMatch('/[\'\""]name[\'\""]\s*=>/'); + }); + + it('reads version from INFO ini file', function () use ($source) { + // setup.php reads version info via parse_ini_file, not a literal array. + expect($source)->toContain('parse_ini_file'); + }); + + it('registers hooks in install function', function () use ($source) { + expect($source)->toContain('api_plugin_register_hook'); + }); +}); diff --git a/tests/Smoke/PluginFilesSyntaxTest.php b/tests/Smoke/PluginFilesSyntaxTest.php new file mode 100644 index 0000000..afe3f30 --- /dev/null +++ b/tests/Smoke/PluginFilesSyntaxTest.php @@ -0,0 +1,109 @@ +getMessage(); + } finally { + restore_error_handler(); + } + + return $error; +} + +describe('plugin file syntax smoke tests', function (): void { + it('main plugin files parse without syntax errors', function (): void { + $root = dirname(__DIR__, 2); + + $files = [ + 'cli/cacti-mapper.php', + 'lib/WeatherMap.class.php', + 'lib/WeatherMap.functions.php', + 'lib/datasources/WeatherMapDataSource_fping.php', + 'lib/datasources/WeatherMapDataSource_rrd.php', + 'lib/poller-common.php', + 'weathermap-cacti-plugin.php', + 'weathermap-cacti-plugin-mgmt.php', + ]; + + foreach ($files as $relfile) { + $path = $root . '/' . $relfile; + + if (!file_exists($path)) { + continue; + } + + $parseError = wm_smoke_check_syntax($path); + + expect($parseError)->toBeNull("Syntax error in {$relfile}: {$parseError}"); + } + }); + + it('setup.php declares the required Cacti plugin hook functions', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/setup.php'); + + expect($source)->toContain('function plugin_weathermap_install('); + expect($source)->toContain('function plugin_weathermap_uninstall('); + expect($source)->toContain('function plugin_weathermap_check_config('); + }); + + it('lib/datasources/ contains at least one WeatherMapDataSource_*.php file', function (): void { + $dir = dirname(__DIR__, 2) . '/lib/datasources'; + $found = glob($dir . '/WeatherMapDataSource_*.php'); + + expect($found)->not->toBeEmpty('No WeatherMapDataSource_*.php files found in lib/datasources/'); + }); + + it('each active datasource file declares a class that extends WeatherMapDataSource', function (): void { + $dir = dirname(__DIR__, 2) . '/lib/datasources'; + $files = glob($dir . '/WeatherMapDataSource_*.php'); + + // Filter out disabled stubs (.php.disabled, .php.txt, etc.). + $active = array_filter((array) $files, fn (string $f): bool => str_ends_with($f, '.php')); + + foreach ($active as $path) { + $source = file_get_contents($path); + + if ($source === false) { + continue; + } + + $relfile = 'lib/datasources/' . basename($path); + + expect($source)->toContain('extends WeatherMapDataSource'); + } + }); +}); diff --git a/tests/Unit/BandwidthPercentageTest.php b/tests/Unit/BandwidthPercentageTest.php new file mode 100644 index 0000000..48cd48b --- /dev/null +++ b/tests/Unit/BandwidthPercentageTest.php @@ -0,0 +1,64 @@ +toBe(50.0); + }); + + it('returns 100.0 at full utilization', function (): void { + expect(calc_bandwidth_percent(1000, 1000))->toBe(100.0); + }); + + it('returns values above 100 when traffic exceeds max', function (): void { + // Clipping is the caller's responsibility; the formula itself is uncapped. + // Use round() to avoid float representation noise (e.g. 110.00000000000001). + expect(round(calc_bandwidth_percent(1100, 1000), 6))->toBe(110.0); + }); + + it('returns 0.0 when max bandwidth is zero', function (): void { + expect(calc_bandwidth_percent(500, 0))->toBe(0.0); + }); + + it('returns 0.0 when traffic is zero', function (): void { + expect(calc_bandwidth_percent(0, 1000))->toBe(0.0); + }); +}); + +describe('calc_bandwidth_percent half-duplex cases', function (): void { + // In half-duplex mode WeatherMap sums in+out before dividing, so the + // combined traffic is passed as a single value. + + it('returns 100.0 when combined in+out equals max', function (): void { + expect(calc_bandwidth_percent(300 + 700, 1000))->toBe(100.0); + }); + + it('returns 0.0 for half-duplex combined traffic when max is zero', function (): void { + expect(calc_bandwidth_percent(300 + 700, 0))->toBe(0.0); + }); +}); + +describe('bandwidth percentage zero guard in source', function (): void { + it('guards against zero max_bandwidth_out before dividing', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.class.php'); + + expect($source)->toContain('($myobj->max_bandwidth_out != 0)'); + expect($source)->toContain('($myobj->max_bandwidth_in != 0)'); + }); +}); diff --git a/tests/Unit/CronFieldOrderTest.php b/tests/Unit/CronFieldOrderTest.php new file mode 100644 index 0000000..3de153e --- /dev/null +++ b/tests/Unit/CronFieldOrderTest.php @@ -0,0 +1,35 @@ +toContain('[$minute, $hour, $day, $month, $wday]'); + }); + + it('does not contain the wrong wday/day transposition', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/poller-common.php'); + + // The historical bug placed $wday before $day. + expect($source)->not->toContain('[$minute, $hour, $wday, $day, $month]'); + }); + + it('maps $minute to tm_min, $hour to tm_hour, $wday to tm_wday, $day to tm_mday, $month to tm_mon', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/poller-common.php'); + + // Verify each field drives the correct localtime key. + expect($source)->toContain("weathermap_cron_part(\$lt['tm_min'], \$minute)"); + expect($source)->toContain("weathermap_cron_part(\$lt['tm_hour'], \$hour)"); + expect($source)->toContain("weathermap_cron_part(\$lt['tm_wday'], \$wday)"); + expect($source)->toContain("weathermap_cron_part(\$lt['tm_mday'], \$day)"); + }); +}); diff --git a/tests/Unit/EditorActionNormalizationTest.php b/tests/Unit/EditorActionNormalizationTest.php new file mode 100644 index 0000000..80052ba --- /dev/null +++ b/tests/Unit/EditorActionNormalizationTest.php @@ -0,0 +1,21 @@ +toContain('function wm_editor_sanitize_action($action, $valid = [])'); + expect($entry)->toContain("'draw'"); + expect($entry)->toContain("'load_map_javascript'"); + }); + + it('normalizes invalid editor actions to empty string at the entrypoint', function (): void { + $entry = file_get_contents(dirname(__DIR__, 2) . '/weathermap-cacti-plugin-editor.php'); + + expect($entry)->toContain("\$action = wm_editor_sanitize_action(get_nfilter_request_var('action'), ["); + expect($entry)->not->toContain("\$action = get_nfilter_request_var('action');"); + }); +}); diff --git a/tests/Unit/FormatNumberTest.php b/tests/Unit/FormatNumberTest.php new file mode 100644 index 0000000..1e3e4ab --- /dev/null +++ b/tests/Unit/FormatNumberTest.php @@ -0,0 +1,16 @@ +toBe('12.35'); + }); + + it('formats whole numbers without a decimal suffix', function (): void { + expect(format_number(12.0, 2))->toBe(12); + }); +}); diff --git a/tests/Unit/RenderColourTest.php b/tests/Unit/RenderColourTest.php new file mode 100644 index 0000000..4b8dfe3 --- /dev/null +++ b/tests/Unit/RenderColourTest.php @@ -0,0 +1,79 @@ +toBe('none'); + }); + + it('returns copy for [-2,-2,-2]', function (): void { + expect(render_colour([-2, -2, -2]))->toBe('copy'); + }); + + it('returns contrast for [-3,-3,-3]', function (): void { + expect(render_colour([-3, -3, -3]))->toBe('contrast'); + }); + + it('formats normal RGB as space-separated integers', function (): void { + expect(render_colour([255, 128, 0]))->toBe('255 128 0'); + }); + + it('formats black as 0 0 0', function (): void { + expect(render_colour([0, 0, 0]))->toBe('0 0 0'); + }); +}); + +describe('render_colour mutation guards (third-element checks)', function (): void { + // These three tests catch the historical bug where $col[2] was written as + // $col[1] in the sentinel conditions, making the third element irrelevant. + + it('does not return none when third element is not -1', function (): void { + expect(render_colour([-1, -1, 0]))->not->toBe('none'); + }); + + it('does not return copy when third element is not -2', function (): void { + expect(render_colour([-2, -2, 0]))->not->toBe('copy'); + }); + + it('does not return contrast when third element is not -3', function (): void { + expect(render_colour([-3, -3, 0]))->not->toBe('contrast'); + }); +}); + +describe('render_colour source correctness', function (): void { + it('uses $col[2] in all three sentinel conditions', function (): void { + $source = file_get_contents(dirname(__DIR__, 2) . '/lib/WeatherMap.functions.php'); + + // All three sentinels must check $col[2], not repeat $col[1]. + expect($source)->toContain('($col[0] == -1) && ($col[1] == -1) && ($col[2] == -1)'); + expect($source)->toContain('($col[0] == -2) && ($col[1] == -2) && ($col[2] == -2)'); + expect($source)->toContain('($col[0] == -3) && ($col[1] == -3) && ($col[2] == -3)'); + }); +}); diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..5e8829e --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,204 @@ + 'db_execute', 'sql' => $sql, 'params' => []]; + + return true; + } +} + +if (!function_exists('db_execute_prepared')) { + function db_execute_prepared($sql, $params = []) { + $GLOBALS['__test_db_calls'][] = ['fn' => 'db_execute_prepared', 'sql' => $sql, 'params' => $params]; + + return true; + } +} + +if (!function_exists('db_fetch_assoc')) { + function db_fetch_assoc($sql) { + return []; + } +} + +if (!function_exists('db_fetch_assoc_prepared')) { + function db_fetch_assoc_prepared($sql, $p = []) { + return []; + } +} + +if (!function_exists('db_fetch_row')) { + function db_fetch_row($sql) { + return []; + } +} + +if (!function_exists('db_fetch_row_prepared')) { + function db_fetch_row_prepared($sql, $p = []) { + return []; + } +} + +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql) { + return ''; + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $p = []) { + return ''; + } +} + +if (!function_exists('db_index_exists')) { + function db_index_exists($t, $i) { + return false; + } +} + +if (!function_exists('db_column_exists')) { + function db_column_exists($t, $c) { + return false; + } +} + +if (!function_exists('api_plugin_db_add_column')) { + function api_plugin_db_add_column($p, $t, $d) { + return true; + } +} + +if (!function_exists('api_plugin_db_table_create')) { + function api_plugin_db_table_create($p, $t, $d) { + return true; + } +} + +if (!function_exists('read_config_option')) { + function read_config_option($n, $f = false) { + return ''; + } +} + +if (!function_exists('set_config_option')) { + function set_config_option($n, $v) { + } +} + +if (!function_exists('html_escape')) { + function html_escape($s) { + return htmlspecialchars($s, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + } +} + +if (!function_exists('__')) { + function __($t, $d = '') { + return $t; + } +} + +if (!function_exists('__esc')) { + function __esc($t, $d = '') { + return htmlspecialchars($t, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + } +} + +if (!function_exists('cacti_log')) { + function cacti_log($m, $p = false, $t = '', $l = 0) { + } +} + +if (!function_exists('cacti_sizeof')) { + function cacti_sizeof($a) { + return is_array($a) ? count($a) : 0; + } +} + +if (!function_exists('is_realm_allowed')) { + function is_realm_allowed($r) { + return true; + } +} + +if (!function_exists('raise_message')) { + function raise_message($i, $t = '', $l = 0) { + } +} + +if (!function_exists('get_request_var')) { + function get_request_var($n) { + return ''; + } +} + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($n) { + return ''; + } +} + +if (!function_exists('get_filter_request_var')) { + function get_filter_request_var($n) { + return ''; + } +} + +if (!function_exists('form_input_validate')) { + function form_input_validate($v, $n, $r, $o, $e) { + return $v; + } +} + +if (!function_exists('is_error_message')) { + function is_error_message() { + return false; + } +} + +if (!function_exists('sql_save')) { + function sql_save($a, $t, $k = 'id') { + return isset($a['id']) ? $a['id'] : 1; + } +} + +if (!defined('CACTI_PATH_BASE')) { + define('CACTI_PATH_BASE', '/var/www/html/cacti'); +} + +if (!defined('POLLER_VERBOSITY_LOW')) { + define('POLLER_VERBOSITY_LOW', 2); +} + +if (!defined('POLLER_VERBOSITY_MEDIUM')) { + define('POLLER_VERBOSITY_MEDIUM', 3); +} + +if (!defined('POLLER_VERBOSITY_DEBUG')) { + define('POLLER_VERBOSITY_DEBUG', 5); +} + +if (!defined('POLLER_VERBOSITY_NONE')) { + define('POLLER_VERBOSITY_NONE', 6); +} + +if (!defined('MESSAGE_LEVEL_ERROR')) { + define('MESSAGE_LEVEL_ERROR', 1); +} + +// Pest v1 has no global describe(); provide a passthrough so test files load. +if (!function_exists('describe')) { + function describe(string $description, Closure $tests): void { + $tests(); + } +} diff --git a/weathermap-cacti-plugin-editor.php b/weathermap-cacti-plugin-editor.php index d7a27b6..421e445 100644 --- a/weathermap-cacti-plugin-editor.php +++ b/weathermap-cacti-plugin-editor.php @@ -1,4 +1,6 @@ ' . __('All', 'weathermap') . ''; - $graph_templates = db_fetch_assoc('SELECT DISTINCT gt.id, gt.name +$graph_templates = db_fetch_assoc('SELECT DISTINCT gt.id, gt.name FROM graph_templates AS gt INNER JOIN graph_local AS gl ON gt.id = gl.graph_template_id ORDER BY gt.name'); - foreach ($graph_templates as $gt) { - print ''; - } - ?> +foreach ($graph_templates as $gt) { + print ''; +} +?> @@ -519,21 +521,21 @@ -
%NODE1%' +
%NODE1%' bits/sec -
%NODE1%' +
%NODE1%' Same As 'In' or bits/sec - + @@ -570,18 +572,18 @@ (no images are available)'; - } else { - print ''; - - foreach ($backgd_list as $im) { - $display = ucfirst(str_replace(['.png', '.gif', '.jpg'], '', basename($im))); - - print ''; - } - } - ?> +if (count($backgd_list) == 0) { + print ''; +} else { + print ''; + + foreach ($backgd_list as $im) { + $display = ucfirst(str_replace(['.png', '.gif', '.jpg'], '', basename($im))); + + print ''; + } +} +?> @@ -730,18 +732,18 @@ diff --git a/weathermap-cacti-plugin-mgmt.php b/weathermap-cacti-plugin-mgmt.php index 1e53b26..966c403 100644 --- a/weathermap-cacti-plugin-mgmt.php +++ b/weathermap-cacti-plugin-mgmt.php @@ -1,4 +1,6 @@