Skip to content

Commit 454e566

Browse files
somethingwithproofsomethingwithproof
committed
test: expand security test coverage for hardening changes
Add targeted tests for prepared statement migration, output escaping, auth guard presence, CSRF token validation, redirect safety, and PHP 7.4 compatibility. Tests use source-scan patterns that verify security invariants without requiring the Cacti database. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent 0ba3429 commit 454e566

9 files changed

Lines changed: 508 additions & 0 deletions

‎composer.json‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
{
2+
"name": "cacti/plugin_hmib",
3+
"description": "plugin_hmib plugin for Cacti",
4+
"license": "GPL-2.0-or-later",
5+
"require-dev": {
6+
"pestphp/pest": "^1.23"
7+
},
8+
"config": {
9+
"allow-plugins": {
10+
"pestphp/pest-plugin": true
11+
}
12+
},
13+
"autoload-dev": {
14+
"files": [
15+
"tests/bootstrap.php"
16+
]
17+
}
18+
}

‎tests/Pest.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDtool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
require_once __DIR__ . '/bootstrap.php';

‎tests/Security/AuthGuardTest.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDtool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
describe('auth guard presence in hmib', function () {
11+
it('includes auth.php or global.php in all UI entry points', function () {
12+
$uiFiles = array(
13+
'hmib.php',
14+
'hmib_types.php',
15+
'snmp.php',
16+
);
17+
18+
foreach ($uiFiles as $relativeFile) {
19+
$path = realpath(__DIR__ . '/../../' . $relativeFile);
20+
if ($path === false) continue;
21+
$contents = file_get_contents($path);
22+
if ($contents === false) continue;
23+
24+
// Files that include setup.php or are library files don't need direct auth
25+
if (strpos($relativeFile, 'include/') === 0 || strpos($relativeFile, 'lib/') === 0) continue;
26+
if (strpos($relativeFile, 'poller_') === 0) continue;
27+
28+
$hasAuth = (
29+
strpos($contents, 'auth.php') !== false ||
30+
strpos($contents, 'global.php') !== false ||
31+
strpos($contents, 'global_arrays.php') !== false
32+
);
33+
34+
expect($hasAuth)->toBeTrue(
35+
"File {$relativeFile} does not include auth.php or global.php"
36+
);
37+
}
38+
});
39+
40+
it('validates numeric IDs from request variables before DB queries', function () {
41+
$uiFiles = array(
42+
'hmib.php',
43+
'hmib_types.php',
44+
'snmp.php',
45+
);
46+
47+
foreach ($uiFiles as $relativeFile) {
48+
$path = realpath(__DIR__ . '/../../' . $relativeFile);
49+
if ($path === false) continue;
50+
$contents = file_get_contents($path);
51+
if ($contents === false) continue;
52+
53+
// Check for get_filter_request_var usage for numeric IDs
54+
if (preg_match('/get_request_var\s*\(\s*['"]id['"]/', $contents)) {
55+
// Should use get_filter_request_var for 'id' params
56+
$hasFilter = (
57+
strpos($contents, 'get_filter_request_var') !== false ||
58+
strpos($contents, 'input_validate_input_number') !== false ||
59+
strpos($contents, 'form_input_validate') !== false
60+
);
61+
62+
expect($hasFilter)->toBeTrue(
63+
"File {$relativeFile} uses get_request_var for IDs without validation"
64+
);
65+
}
66+
}
67+
});
68+
});

‎tests/Security/OutputEscapingTest.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDtool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
describe('output escaping in hmib', function () {
11+
it('does not interpolate raw variables into HTML attributes', function () {
12+
$uiFiles = array(
13+
'hmib.php',
14+
'hmib_types.php',
15+
'snmp.php',
16+
);
17+
18+
foreach ($uiFiles as $relativeFile) {
19+
$path = realpath(__DIR__ . '/../../' . $relativeFile);
20+
if ($path === false) continue;
21+
$contents = file_get_contents($path);
22+
if ($contents === false) continue;
23+
24+
$lines = explode("\n", $contents);
25+
$dangerous = 0;
26+
27+
foreach ($lines as $line) {
28+
$trimmed = ltrim($line);
29+
if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0) continue;
30+
31+
// value="$row[...] without html_escape wrapping
32+
if (preg_match('/value\s*=\s*["\'"]\s*<\?php\s+echo\s+\$/', $line)) {
33+
$dangerous++;
34+
}
35+
// title="<?php print $something without escaping
36+
if (preg_match('/(?:title|alt|placeholder)\s*=.*print\s+\$(?!_|config)/', $line)) {
37+
if (strpos($line, 'html_escape') === false && strpos($line, '__esc') === false && strpos($line, 'htmlspecialchars') === false) {
38+
$dangerous++;
39+
}
40+
}
41+
}
42+
43+
expect($dangerous)->toBe(0,
44+
"File {$relativeFile} has unescaped variables in HTML attributes"
45+
);
46+
}
47+
});
48+
49+
it('uses html_escape or __esc for user-controlled output', function () {
50+
$uiFiles = array(
51+
'hmib.php',
52+
'hmib_types.php',
53+
'snmp.php',
54+
);
55+
56+
$totalEscapeCalls = 0;
57+
58+
foreach ($uiFiles as $relativeFile) {
59+
$path = realpath(__DIR__ . '/../../' . $relativeFile);
60+
if ($path === false) continue;
61+
$contents = file_get_contents($path);
62+
if ($contents === false) continue;
63+
64+
$totalEscapeCalls += preg_match_all('/html_escape|__esc\(|htmlspecialchars/', $contents);
65+
}
66+
67+
// At least some escaping should be present in UI files
68+
expect($totalEscapeCalls)->toBeGreaterThan(0,
69+
'UI files should contain at least one html_escape/__esc call'
70+
);
71+
});
72+
});

‎tests/Security/Php74CompatibilityTest.php‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDtool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
describe('PHP 7.4 compatibility in hmib', function () {
11+
$files = array(
12+
'hmib.php',
13+
'hmib_types.php',
14+
'poller_graphs.php',
15+
'poller_hmib.php',
16+
'snmp.php',
17+
);
18+
19+
it('does not use str_contains (PHP 8.0)', function () use ($files) {
20+
foreach ($files as $f) {
21+
$p = realpath(__DIR__ . '/../../' . $f);
22+
if ($p === false) continue;
23+
$c = file_get_contents($p);
24+
if ($c === false) continue;
25+
expect(preg_match('/\bstr_contains\s*\(/', $c))->toBe(0, "{$f} uses str_contains");
26+
}
27+
});
28+
29+
it('does not use str_starts_with (PHP 8.0)', function () use ($files) {
30+
foreach ($files as $f) {
31+
$p = realpath(__DIR__ . '/../../' . $f);
32+
if ($p === false) continue;
33+
$c = file_get_contents($p);
34+
if ($c === false) continue;
35+
expect(preg_match('/\bstr_starts_with\s*\(/', $c))->toBe(0, "{$f} uses str_starts_with");
36+
}
37+
});
38+
39+
it('does not use str_ends_with (PHP 8.0)', function () use ($files) {
40+
foreach ($files as $f) {
41+
$p = realpath(__DIR__ . '/../../' . $f);
42+
if ($p === false) continue;
43+
$c = file_get_contents($p);
44+
if ($c === false) continue;
45+
expect(preg_match('/\bstr_ends_with\s*\(/', $c))->toBe(0, "{$f} uses str_ends_with");
46+
}
47+
});
48+
49+
it('does not use nullsafe operator (PHP 8.0)', function () use ($files) {
50+
foreach ($files as $f) {
51+
$p = realpath(__DIR__ . '/../../' . $f);
52+
if ($p === false) continue;
53+
$c = file_get_contents($p);
54+
if ($c === false) continue;
55+
expect(preg_match('/\?->/', $c))->toBe(0, "{$f} uses nullsafe operator");
56+
}
57+
});
58+
59+
it('does not use match expression (PHP 8.0)', function () use ($files) {
60+
foreach ($files as $f) {
61+
$p = realpath(__DIR__ . '/../../' . $f);
62+
if ($p === false) continue;
63+
$c = file_get_contents($p);
64+
if ($c === false) continue;
65+
// Avoid false positive on preg_match etc
66+
$c2 = preg_replace('/preg_match|preg_match_all|fnmatch/', '', $c);
67+
expect(preg_match('/\bmatch\s*\(/', $c2))->toBe(0, "{$f} uses match expression");
68+
}
69+
});
70+
71+
it('does not use union type declarations (PHP 8.0)', function () use ($files) {
72+
foreach ($files as $f) {
73+
$p = realpath(__DIR__ . '/../../' . $f);
74+
if ($p === false) continue;
75+
$c = file_get_contents($p);
76+
if ($c === false) continue;
77+
// Match function params/return with union types like string|false
78+
$hits = preg_match_all('/function\s+\w+\s*\([^)]*\w+\s*\|\s*\w+/', $c);
79+
expect($hits)->toBe(0, "{$f} uses union types in function signatures");
80+
}
81+
});
82+
83+
it('does not use constructor property promotion (PHP 8.0)', function () use ($files) {
84+
foreach ($files as $f) {
85+
$p = realpath(__DIR__ . '/../../' . $f);
86+
if ($p === false) continue;
87+
$c = file_get_contents($p);
88+
if ($c === false) continue;
89+
expect(preg_match('/function\s+__construct\s*\([^)]*\b(public|private|protected|readonly)\s/', $c))->toBe(0,
90+
"{$f} uses constructor promotion"
91+
);
92+
}
93+
});
94+
95+
it('uses array() not short syntax for new arrays', function () use ($files) {
96+
// This is a style preference for 1.2.x consistency, not a hard requirement
97+
// Just verify no mixed styles in the same file
98+
foreach ($files as $f) {
99+
$p = realpath(__DIR__ . '/../../' . $f);
100+
if ($p === false) continue;
101+
$c = file_get_contents($p);
102+
if ($c === false) continue;
103+
104+
$hasArrayFunc = preg_match('/\barray\s*\(/', $c);
105+
$hasShortArray = preg_match('/=\s*\[/', $c);
106+
107+
// Flag files that mix both styles
108+
if ($hasArrayFunc && $hasShortArray) {
109+
// Allow mixed if the file existed before our changes
110+
// This is informational, not a hard fail
111+
}
112+
}
113+
114+
expect(true)->toBeTrue();
115+
});
116+
});

‎tests/Security/PreparedStatementConsistencyTest.php‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDtool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
describe('prepared statement consistency in hmib', function () {
11+
it('uses prepared DB helpers in all plugin files', function () {
12+
$targetFiles = array(
13+
'hmib.php',
14+
'hmib_types.php',
15+
'poller_graphs.php',
16+
'poller_hmib.php',
17+
'snmp.php',
18+
);
19+
20+
$rawPattern = '/\bdb_(?:execute|fetch_row|fetch_assoc|fetch_cell)\s*\(/';
21+
$preparedPattern = '/\bdb_(?:execute|fetch_row|fetch_assoc|fetch_cell)_prepared\s*\(/';
22+
23+
foreach ($targetFiles as $relativeFile) {
24+
$path = realpath(__DIR__ . '/../../' . $relativeFile);
25+
if ($path === false) continue;
26+
$contents = file_get_contents($path);
27+
if ($contents === false) continue;
28+
29+
$lines = explode("\n", $contents);
30+
$rawCalls = 0;
31+
32+
foreach ($lines as $line) {
33+
$trimmed = ltrim($line);
34+
if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0 || strpos($trimmed, '#') === 0) continue;
35+
if (preg_match($rawPattern, $line) && !preg_match($preparedPattern, $line)) {
36+
$rawCalls++;
37+
}
38+
}
39+
40+
expect($rawCalls)->toBe(0, "File {$relativeFile} contains raw DB calls");
41+
}
42+
});
43+
44+
it('uses parameterized placeholders not string interpolation in SQL', function () {
45+
$targetFiles = array(
46+
'hmib.php',
47+
'hmib_types.php',
48+
'poller_graphs.php',
49+
'poller_hmib.php',
50+
'snmp.php',
51+
);
52+
53+
foreach ($targetFiles as $relativeFile) {
54+
$path = realpath(__DIR__ . '/../../' . $relativeFile);
55+
if ($path === false) continue;
56+
$contents = file_get_contents($path);
57+
if ($contents === false) continue;
58+
59+
$lines = explode("\n", $contents);
60+
$interpolatedSql = 0;
61+
62+
foreach ($lines as $num => $line) {
63+
$trimmed = ltrim($line);
64+
if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0) continue;
65+
66+
// Detect _prepared calls with $ interpolation instead of ? placeholders
67+
if (preg_match('/_prepared\s*\(/', $line) && preg_match('/\$[a-zA-Z_]/', $line)) {
68+
// Allow array($var) param binding but flag "WHERE id = $var"
69+
if (preg_match('/(?:SELECT|INSERT|UPDATE|DELETE|WHERE|SET|FROM|JOIN).*\$/', $line)) {
70+
$interpolatedSql++;
71+
}
72+
}
73+
}
74+
75+
// This is a heuristic; some false positives expected for complex queries
76+
expect($interpolatedSql)->toBeLessThanOrEqual(2,
77+
"File {$relativeFile} may have SQL interpolation in prepared calls"
78+
);
79+
}
80+
});
81+
});

0 commit comments

Comments
 (0)