From c50565cbb7236776616bae0febbe323c092ac4b0 Mon Sep 17 00:00:00 2001 From: Sean Mancini Date: Mon, 17 Aug 2026 16:14:35 -0400 Subject: [PATCH] security: replace rand() with hrtime(true) for graph cache-buster (S2245) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CHANGELOG.md | 1 + tests/Unit/GraphCacheBusterTest.php | 62 +++++++++++++++++++++++++++++ thold.php | 2 +- 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 tests/Unit/GraphCacheBusterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index ae48cd43..903a6084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ * issue#719: Plugin Disabled due to mix of string and int * issue: All Columns checkd on Thresholds page * issue: Special character previous value handling broken on data query indexes with special characters +* security: Replace rand() with hrtime(true) for graph image cache-buster (GHSA-vhwj-hfwg-gfg3, CWE-338) --- 1.8.2 --- diff --git a/tests/Unit/GraphCacheBusterTest.php b/tests/Unit/GraphCacheBusterTest.php new file mode 100644 index 00000000..794224a8 --- /dev/null +++ b/tests/Unit/GraphCacheBusterTest.php @@ -0,0 +1,62 @@ +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); + } +} \ No newline at end of file diff --git a/thold.php b/thold.php index e9c491c6..7572085f 100644 --- a/thold.php +++ b/thold.php @@ -1275,7 +1275,7 @@ function thold_edit() {
- '> + '>