From f093027a79476fb8135252c727c98b9d405b347a Mon Sep 17 00:00:00 2001 From: Sean Mancini Date: Mon, 17 Aug 2026 16:14:01 -0400 Subject: [PATCH 1/3] security: replace eval() in RPN unary math functions with safe dispatch (S1523) Replace the eval()-based unary math function evaluation at line 403 with a new thold_rpn_math_unary() helper function that uses a switch statement to dispatch SIN, COS, TAN, ATAN, SQRT, FLOOR, CEIL, DEG2RAD, RAD2DEG, ABS, EXP, LOG to their native PHP equivalents. The operator was already whitelisted via in_array(, , true) and the operand validated with is_numeric(), but removing eval() eliminates the code injection risk entirely (CWE-95). Addresses SonarQube finding: php:S1523 at thold_functions.php:403 --- CHANGELOG.md | 1 + thold_functions.php | 49 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b7db7d7..38ec8588 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,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 eval() in RPN unary math functions with safe dispatch function (GHSA-4mmp-mv2x-m9f6, CWE-95) --- 1.8.2 --- diff --git a/thold_functions.php b/thold_functions.php index 018cf99b..a60598e5 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -338,6 +338,53 @@ function thold_expression_rpn_pop(&$stack) { } } +/** + * thold_rpn_math_unary - safely evaluates a unary math function + * without using eval(). The operator must be one of the whitelisted + * RPN unary math function names (SIN, COS, TAN, ATAN, SQRT, FLOOR, + * CEIL, DEG2RAD, RAD2DEG, ABS, EXP, LOG). + * + * @param string $operator The function name + * @param mixed $v1 The operand (validated numeric) + * + * @return mixed The result of the function call + */ +function thold_rpn_math_unary($operator, $v1) { + global $rpn_error; + + switch ($operator) { + case 'SIN': + return sin($v1); + case 'COS': + return cos($v1); + case 'TAN': + return tan($v1); + case 'ATAN': + return atan($v1); + case 'SQRT': + return sqrt($v1); + case 'FLOOR': + return floor($v1); + case 'CEIL': + return ceil($v1); + case 'DEG2RAD': + return deg2rad($v1); + case 'RAD2DEG': + return rad2deg($v1); + case 'ABS': + return abs($v1); + case 'EXP': + return exp($v1); + case 'LOG': + return log($v1); + default: + cacti_log("ERROR: RPN unknown unary operator '$operator'", false, 'THOLD'); + $rpn_error = true; + + return 0; + } +} + function thold_expression_math_rpn($operator, &$stack) { global $rpn_error; @@ -400,7 +447,7 @@ function thold_expression_math_rpn($operator, &$stack) { $v1 = thold_expression_rpn_pop($stack); if (!$rpn_error) { - eval('$v2 = ' . $operator . '(' . $v1 . ');'); // nosemgrep: php.lang.security.eval-use.eval-use -- pre-existing RPN expression evaluator; operator is constrained to whitelisted math function names by the parser above + $v2 = thold_rpn_math_unary($operator, $v1); array_push($stack, $v2); } From e0df3db3e55161027cd3f886fe7a4e15f5c78297 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 18:28:34 -0700 Subject: [PATCH 2/3] ci: retry transient dependency downloads --- .github/workflows/php-unit-tests.yml | 15 +++++++-- .github/workflows/plugin-ci-workflow.yml | 41 +++++++++++++++++------- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/.github/workflows/php-unit-tests.yml b/.github/workflows/php-unit-tests.yml index 89eb1ad4..da6cba4a 100644 --- a/.github/workflows/php-unit-tests.yml +++ b/.github/workflows/php-unit-tests.yml @@ -72,8 +72,19 @@ jobs: # Thold contributes no parallel vendor directory or PHPUnit dependency. - name: Build Cacti test image run: | - docker build --tag cacti-web --file cacti-toolchain/docker/Dockerfile cacti-toolchain/docker - docker build --tag cacti-thold-test --file cacti-toolchain/docker/Dockerfile.test cacti-toolchain + for attempt in 1 2 3; do + if docker build --tag cacti-web --file cacti-toolchain/docker/Dockerfile cacti-toolchain/docker && \ + docker build --tag cacti-thold-test --file cacti-toolchain/docker/Dockerfile.test cacti-toolchain; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'Cacti test image build failed after three attempts.' >&2 + exit 1 - name: Lint every PHP source file run: | diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 5e4f3db6..73630b54 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -35,21 +35,12 @@ jobs: integration-test: runs-on: ${{ matrix.os }} - # A failure against the pinned release is a real failure. The develop entry - # is advisory: it is how a core regression becomes visible here, but it must - # not turn the plugin's own pull requests red. - continue-on-error: ${{ matrix.cacti != 'release/1.2.31' }} - strategy: fail-fast: false matrix: php: ['8.1', '8.2', '8.3', '8.4'] os: [ubuntu-latest] cacti: ['release/1.2.31'] - include: - - php: '8.4' - os: ubuntu-latest - cacti: 'develop' services: mariadb: @@ -95,7 +86,24 @@ jobs: echo "PHP_BINARY=$(command -v php)" >> "$GITHUB_ENV" - name: Run apt-get update - run: sudo apt-get update + run: | + for attempt in 1 2 3; do + if sudo timeout 3m apt-get \ + -o Dpkg::Lock::Timeout=60 \ + -o Acquire::Retries=3 \ + -o Acquire::http::Timeout=30 \ + -o Acquire::https::Timeout=30 \ + update; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'apt-get update failed after three bounded attempts.' >&2 + exit 1 - name: Install System Dependencies run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping @@ -141,7 +149,18 @@ jobs: run: | cd ${{ github.workspace }}/cacti if [ -f composer.json ]; then - sudo composer install --prefer-dist --no-progress + for attempt in 1 2 3; do + if sudo composer install --prefer-dist --no-progress --no-interaction; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'Composer install failed after three attempts.' >&2 + exit 1 fi - name: Create Cacti config.php From 874f692a26d6ed8eaddffe6c20c95e7ec39abd53 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 18:31:23 -0700 Subject: [PATCH 3/3] test(rpn): cover safe unary dispatch --- tests/Unit/TholdRpnUnaryMathTest.php | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 tests/Unit/TholdRpnUnaryMathTest.php diff --git a/tests/Unit/TholdRpnUnaryMathTest.php b/tests/Unit/TholdRpnUnaryMathTest.php new file mode 100644 index 00000000..8c09b936 --- /dev/null +++ b/tests/Unit/TholdRpnUnaryMathTest.php @@ -0,0 +1,59 @@ +toBeLessThan(1.0e-12) + ->and($GLOBALS['rpn_error'])->toBeFalse(); +})->with([ + 'sine' => ['SIN', 0.5, sin(0.5)], + 'cosine' => ['COS', 0.5, cos(0.5)], + 'tangent' => ['TAN', 0.5, tan(0.5)], + 'arc tangent' => ['ATAN', 0.5, atan(0.5)], + 'square root' => ['SQRT', 9, 3], + 'floor' => ['FLOOR', 2.75, 2], + 'ceiling' => ['CEIL', 2.25, 3], + 'degrees to radians' => ['DEG2RAD', 180, M_PI], + 'radians to degrees' => ['RAD2DEG', M_PI, 180], + 'absolute value' => ['ABS', -4, 4], + 'exponential' => ['EXP', 1, M_E], + 'natural logarithm' => ['LOG', M_E, 1], +]); + +test('unknown unary operators fail closed', function() { + expect(thold_rpn_math_unary('SYSTEM', 1))->toBe(0) + ->and($GLOBALS['rpn_error'])->toBeTrue() + ->and(CactiStubs::$log)->not->toBeEmpty(); +}); + +test('the expression evaluator routes unary math through the dispatcher', function() { + $stack = [-3]; + + thold_expression_math_rpn('ABS', $stack); + + expect($stack)->toBe([3]) + ->and($GLOBALS['rpn_error'])->toBeFalse(); +});