mirror of
https://github.com/Cacti/plugin_thold.git
synced 2026-10-23 03:27:05 +09:00
* security: replace rand() with hrtime(true) for graph cache-buster (S2245)
Replaces the non-cryptographic PRNG rand() with hrtime(true), a
monotonic nanosecond clock that never throws. This URL parameter is
a cache-buster, not a security boundary, so a CSPRNG (random_int)
is the wrong reliability tradeoff — it can throw Random\RandomException
on CSPRNG failure, making threshold editing fatal.
hrtime(true) provides:
- Never throws (no CSPRNG dependency)
- Monotonic (never goes backwards on NTP step)
- Nanosecond resolution (no collisions on fast page renders)
- Returns int (clean URL query parameter)
Fixes GHSA-vhwj-hfwg-gfg3
Rule: php:S2245 (CWE-338)
* fix: restore SQL binding, trigger-command quoting, and RPN safety fixes
These security/robustness fixes were already documented in CHANGELOG.md and
covered by tests (GetAllowedThresholdsTest, TholdCommandExecutionTest,
TholdReplaceThresholdTagsTest, TholdExpressionMathRpnTest,
OptionalCoreFunctionTest), but the corresponding thold_functions.php changes
were missing, leaving the Pest suite failing on every PR built on develop.
- get_allowed_thresholds()/get_allowed_threshold_logs(): bind graph_id and
caller-supplied params instead of interpolating them into the SQL text,
and use the *_prepared fetchers.
- thold_replace_threshold_tags(): add a \ flag that quotes
device/threshold free-text values with cacti_escapeshellarg() when the
substituted text is a trigger command; email/notification callers are
unaffected.
- thold_command_execution(): pass shell=true when building the three
trigger commands, and fix a topic-string typo ('thold' vs 'thold_cmd')
that silently suppressed exit-status logging for inline (non-queued)
command runs.
- thold_expression_math_rpn(): remove a stray break that exited the
operator switch before pushing the 0/0 result, flag modulo-by-zero
instead of letting PHP throw DivisionByZeroError, reject non-numeric
unary operands before eval(), and flag SQRT/LOG results that are NAN or
INF instead of pushing them onto the stack.
* fix(tests): stop includes/arrays.php loading from silently no-op'ing
thold_functions.php includes includes/arrays.php with a plain include()
(not include_once()) from several of its own functions (e.g. thold_log()),
keyed off \['base_path']. Once any test in the shared Pest process
exercised one of those call sites, PHP's include-once registry considered
the file already included: a later thold_test_load() (require_once) on the
same resolved path silently no-op'd and never (re)published \
to \, depending on which order the test files happened to run in.
This intermittently broke every test that reads \
(TholdReplaceThresholdTagsTest's THOLDTYPE substitution) as well as tests
several call frames downstream of thold_log()/thold_check_threshold()
(NotificationEmailDeduplicationTest, ThresholdTimeBasedCharacterizationTest),
depending on file execution order.
Add thold_test_load_always()/loadPluginSourceAlways(), which use a plain
require() instead of require_once(), for plugin files that only assign
file-scope variables (no function/class declarations) and are therefore
safe to load more than once. Switch the four test classes that load
includes/arrays.php to the new helper.
* debug: temporary CI diagnostics for notification/restoral test failures
Adds stderr diagnostics behind THOLD_CI_DEBUG to see the real values
computed on the Linux Pest CI run for the two failures that do not
reproduce locally on Windows. Will be removed once the root cause is
identified.
* debug: make CI diagnostics fire unconditionally for this run
* Revert "debug: make CI diagnostics fire unconditionally for this run"
This reverts commit 71877471e7.
* Revert "debug: temporary CI diagnostics for notification/restoral test failures"
This reverts commit 76d05ad300.
* debug: temporary CI diagnostics (round 2)
* chore: retrigger CI
* Revert "debug: temporary CI diagnostics (round 2)"
This reverts commit 7ce80b2d1c.
* debug: temporary CI diagnostics (round 3)
* chore: retrigger CI for diagnostics
* Revert "debug: temporary CI diagnostics (round 3)"
This reverts commit 9aff7a428e.
* debug: temporary CI diagnostics (round 4)
* chore: retrigger CI (2)
* Revert "debug: temporary CI diagnostics (round 4)"
This reverts commit 4bb7f44ecf.
* debug: temporary CI diagnostics (round 5)
* chore: retrigger CI (3)
---------
Co-authored-by: TheWitness <thewitness@cacti.net>
62 lines
2.1 KiB
PHP
62 lines
2.1 KiB
PHP
<?php
|
|
/*
|
|
+-------------------------------------------------------------------------+
|
|
| 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. |
|
|
+-------------------------------------------------------------------------+
|
|
| Cacti: The Complete RRDTool-based Graphing Solution |
|
|
+-------------------------------------------------------------------------+
|
|
| http://www.cacti.net/ |
|
|
+-------------------------------------------------------------------------+
|
|
*/
|
|
|
|
/**
|
|
* The graph image cache-buster on the threshold edit page must never make
|
|
* editing fatal. random_int() throws Random\RandomException on CSPRNG
|
|
* failure; mt_rand() does not throw and is the correct tool for a
|
|
* non-security cache-buster.
|
|
*/
|
|
final class GraphCacheBusterTest extends TestCase {
|
|
/**
|
|
* @return void
|
|
*/
|
|
public function testMtRandDoesNotThrowAndProducesInteger(): void {
|
|
// mt_rand() must not throw under any circumstance
|
|
$value = mt_rand();
|
|
|
|
$this->assertIsInt($value);
|
|
$this->assertGreaterThan(0, $value);
|
|
}
|
|
|
|
/**
|
|
* @return void
|
|
*/
|
|
public function testMtRandProducesVaryingValuesAcrossCalls(): void {
|
|
$values = [];
|
|
|
|
for ($i = 0; $i < 100; $i++) {
|
|
$values[] = mt_rand();
|
|
}
|
|
|
|
// At least two distinct values in 100 calls — cache-busting requires variation
|
|
$this->assertGreaterThan(1, count(array_unique($values)));
|
|
}
|
|
|
|
/**
|
|
* The cache-buster is embedded in an HTML img src attribute via
|
|
* html_escape(). Confirm the value round-trips safely.
|
|
*
|
|
* @return void
|
|
*/
|
|
public function testCacheBusterValueIsHtmlSafe(): void {
|
|
$value = mt_rand();
|
|
|
|
$escaped = html_escape((string) $value);
|
|
|
|
$this->assertSame((string) $value, $escaped);
|
|
}
|
|
} |