Skip to content

Commit c50565c

Browse files
bmfmancinisomethingwithproof
authored andcommitted
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)
1 parent 5799446 commit c50565c

3 files changed

Lines changed: 64 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
* issue#719: Plugin Disabled due to mix of string and int
2121
* issue: All Columns checkd on Thresholds page
2222
* issue: Special character previous value handling broken on data query indexes with special characters
23+
* security: Replace rand() with hrtime(true) for graph image cache-buster (GHSA-vhwj-hfwg-gfg3, CWE-338)
2324

2425
--- 1.8.2 ---
2526

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| This program is free software; you can redistribute it and/or |
7+
| modify it under the terms of the GNU General Public License |
8+
| as published by the Free Software Foundation; either version 2 |
9+
| of the License, or (at your option) any later version. |
10+
+-------------------------------------------------------------------------+
11+
| Cacti: The Complete RRDTool-based Graphing Solution |
12+
+-------------------------------------------------------------------------+
13+
| http://www.cacti.net/ |
14+
+-------------------------------------------------------------------------+
15+
*/
16+
17+
/**
18+
* The graph image cache-buster on the threshold edit page must never make
19+
* editing fatal. random_int() throws Random\RandomException on CSPRNG
20+
* failure; mt_rand() does not throw and is the correct tool for a
21+
* non-security cache-buster.
22+
*/
23+
final class GraphCacheBusterTest extends TestCase {
24+
/**
25+
* @return void
26+
*/
27+
public function testMtRandDoesNotThrowAndProducesInteger(): void {
28+
// mt_rand() must not throw under any circumstance
29+
$value = mt_rand();
30+
31+
$this->assertIsInt($value);
32+
$this->assertGreaterThan(0, $value);
33+
}
34+
35+
/**
36+
* @return void
37+
*/
38+
public function testMtRandProducesVaryingValuesAcrossCalls(): void {
39+
$values = [];
40+
41+
for ($i = 0; $i < 100; $i++) {
42+
$values[] = mt_rand();
43+
}
44+
45+
// At least two distinct values in 100 calls — cache-busting requires variation
46+
$this->assertGreaterThan(1, count(array_unique($values)));
47+
}
48+
49+
/**
50+
* The cache-buster is embedded in an HTML img src attribute via
51+
* html_escape(). Confirm the value round-trips safely.
52+
*
53+
* @return void
54+
*/
55+
public function testCacheBusterValueIsHtmlSafe(): void {
56+
$value = mt_rand();
57+
58+
$escaped = html_escape((string) $value);
59+
60+
$this->assertSame((string) $value, $escaped);
61+
}
62+
}

thold.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1275,7 +1275,7 @@ function thold_edit() {
12751275
<br>
12761276
</td>
12771277
<td class='textArea' style='vertical-align:middle;padding:5px'>
1278-
<img id='graphimage' src='<?php print html_escape($config['url_path'] . 'graph_image.php?local_graph_id=' . $thold_data['local_graph_id'] . '&rra_id=0&graph_start=' . $timespan['begin_now'] . '&graph_end=' . $timespan['end_now'] . '&graph_height=150&graph_width=600&randome=' . rand()); ?>'>
1278+
<img id='graphimage' src='<?php print html_escape($config['url_path'] . 'graph_image.php?local_graph_id=' . $thold_data['local_graph_id'] . '&rra_id=0&graph_start=' . $timespan['begin_now'] . '&graph_end=' . $timespan['end_now'] . '&graph_height=150&graph_width=600&randome=' . hrtime(true)); ?>'>
12791279
</td>
12801280
</tr>
12811281
<?php

0 commit comments

Comments
 (0)