1 Commits
Author SHA1 Message Date
Sean ManciniandTheWitness 2288c7a1ee security: replace rand() with hrtime(true) for graph cache-buster (GHSA-vhwj-hfwg-gfg3) (#806)
* 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>
2026-09-21 10:23:07 -04:00