Files
cacti/tests/Unit/DataCollection/Poller/ProcessPidReuseGuardTest.php
Thomas Vincent 3f377531b7 fix(poller): stop the registry pid probe narrowing a wide pid to -1 (#7956)
* fix(poller): stop the registry pid probe narrowing a wide pid to -1

processes.pid is int(10) unsigned, so a corrupted row can hold 4294967295.
Through PHP 8.4 posix_kill() narrows that to a 32 bit pid_t of -1 and kill(2)
reads it as every process the caller may signal, so cacti_process_pid_exists()
calls a dead registry row live and the two kill sites that ask it first SIGTERM
the caller's whole session. From PHP 8.5 the same value raises a ValueError and
ends the poller instead. Refusing it ahead of the call covers both.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* hardening: bound every process table PID before signalling it

Twelve call sites read a pid out of a process table and handed it straight to
posix_kill(). processes.pid is int(10) unsigned, so a corrupted row can carry
4294967295, which narrows to a 32 bit pid_t of -1 and signals every process
the caller owns, or from PHP 8.5 raises a ValueError that ends the collector.
They now go through cacti_process_kill(), which refuses init, anything that
cannot name a process, and anything wider than pid_t.

poller_recovery read its pid with a bare posix_kill($pid, 0) and treats false
as "stale, clear the row and run". cacti_process_signalable() keeps exactly
that meaning, including EPERM as stale, and adds the bound. Neither
cacti_process_pid_exists() nor cacti_process_still_running() fits there: both
report a pid live when signalling it is merely forbidden, so a recycled
recovery_pid would have wedged recovery until an operator cleared the row.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test: place process guards in poller domain

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(poller): honor platform PID ranges

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(poller): cover platform signal semantics

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
2026-09-12 15:24:27 -04:00

266 lines
10 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. |
| |
| This program is distributed in the hope that it will be useful, |
| but WITHOUT ANY WARRANTY; without even the implied warranty of |
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
| GNU General Public License for more details. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/*
* The process registry stored a pid and nothing else, and every caller tested
* it with a bare posix_kill($pid, 0). That only proves some process owns the
* pid. Once a registered process dies without unregistering, the system is free
* to hand its pid to an unrelated program, and Cacti would then either refuse
* to start a legitimate task or send SIGTERM to a stranger.
*
* cacti_process_still_running() adds an identity check on top of the existence
* check, using the command name under /proc where that exists.
*/
require_once dirname(__DIR__, 4) . '/include/global_constants.php';
require_once dirname(__DIR__, 4) . '/lib/poller.php';
/**
* Starts a process running a different program and returns its pid.
*
* @param resource|null $handle Receives the process handle, to close later.
*
* @return int The pid of the started process.
*/
function pid_guard_spawn_foreign(&$handle) : int {
$pipes = [];
// array form, so the child is sleep itself rather than a wrapping shell
$handle = proc_open(['sleep', '30'], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
$status = proc_get_status($handle);
$pid = (int) $status['pid'];
foreach ($pipes as $pipe) {
fclose($pipe);
}
/* proc_open forks, then the child execs sleep. Until the exec lands, the
child still reports the parent's command name, so an identity check that
runs in that window would see a false match. Wait for the exec to settle
before returning so the test observes 'sleep', not the transient 'php'. */
$comm = '/proc/' . $pid . '/comm';
for ($i = 0; $i < 200; $i++) {
if (is_readable($comm) && trim((string) file_get_contents($comm)) === 'sleep') {
break;
}
usleep(5000);
}
return $pid;
}
test('a pid that cannot be valid is never reported running', function () {
expect(cacti_process_still_running(0))->toBeFalse()
->and(cacti_process_still_running(-1))->toBeFalse()
->and(cacti_process_still_running(-99999))->toBeFalse();
});
test('a pid wider than pid_t is never reported existing or running', function () {
if (PHP_INT_SIZE < 8) {
test()->markTestSkipped('a 32 bit build cannot express a pid wider than pid_t');
}
/* 4294967295 is the largest value processes.pid can hold. Handing it to an
unguarded posix_kill() is unsafe in two different ways: through PHP 8.4
the argument narrows to a 32 bit pid_t of -1, which kill(2) reads as every
process the caller may signal, and from PHP 8.5 the same call raises a
ValueError that takes the poller down instead. Refusing the value ahead of
the call covers both, so this asserts the refusal rather than whichever
behaviour the running interpreter would have produced. */
expect(cacti_process_pid_exists(4294967295))->toBeFalse()
->and(cacti_process_still_running(4294967295))->toBeFalse()
->and(cacti_process_pid_exists(PHP_INT_MAX))->toBeFalse()
->and(cacti_process_still_running(PHP_INT_MAX))->toBeFalse();
});
test('every value pid_t can hold is still answered by the kernel', function () {
/* The bound sits at pid_t and not below it, so the largest pid_t is still
passed to posix_kill() and a pid that does exist is still recognised. */
expect(posix_kill(2147483647, 0))->toBeFalse()
->and(cacti_process_pid_exists(2147483647))->toBeFalse()
->and(cacti_process_pid_exists(getmypid()))->toBeTrue();
});
test('the running process recognises itself', function () {
expect(cacti_process_still_running(getmypid()))->toBeTrue();
});
test('a permissions failure still means the pid exists', function () {
$eperm = defined('SOCKET_EPERM') ? SOCKET_EPERM : 1;
if (posix_kill(1, 0) || posix_get_last_error() !== $eperm) {
test()->markTestSkipped('pid 1 does not produce EPERM for this user');
}
expect(cacti_process_pid_exists(1))->toBeTrue();
});
test('a pid that has already exited is not reported running', function () {
$pipes = [];
$handle = proc_open(['true'], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
$pid = (int) proc_get_status($handle)['pid'];
foreach ($pipes as $pipe) {
fclose($pipe);
}
proc_close($handle);
expect(cacti_process_still_running($pid))->toBeFalse();
});
/**
* The identity half of the check, which is the whole point of the fix. It needs
* the command names under /proc, so it only means anything on Linux.
*/
test('a live pid belonging to a different program is not reported running', function () {
if (!is_dir('/proc')) {
test()->markTestSkipped('no /proc, so command names cannot be compared');
}
if (PHP_SAPI !== 'cli') {
test()->markTestSkipped('the identity check only applies to CLI callers');
}
$handle = null;
$pid = pid_guard_spawn_foreign($handle);
if (!is_readable('/proc/' . getmypid() . '/comm') || !is_readable('/proc/' . $pid . '/comm')) {
proc_terminate($handle);
proc_close($handle);
test()->markTestSkipped('process command names are not readable under /proc');
}
// the pid is alive, so the bare existence test the fix replaced said yes
expect(posix_kill($pid, 0))->toBeTrue()
->and(cacti_process_still_running($pid))->toBeFalse();
proc_terminate($handle);
proc_close($handle);
});
/**
* Where /proc is absent the function is documented to fall back to the bare
* existence test, so this pins that it stays no worse than before rather than
* silently reporting healthy processes dead.
*/
test('without /proc the check falls back to plain existence', function () {
if (is_dir('/proc')) {
test()->markTestSkipped('/proc is present, so the fallback does not apply');
}
$handle = null;
$pid = pid_guard_spawn_foreign($handle);
expect(cacti_process_still_running($pid))->toBeTrue();
proc_terminate($handle);
proc_close($handle);
});
test('every registry kill site checks the pid before signalling it', function () {
$poller = file_get_contents(dirname(__DIR__, 4) . '/lib/poller.php');
$dsstats = file_get_contents(dirname(__DIR__, 4) . '/lib/dsstats.php');
$batchgapfix = file_get_contents(dirname(__DIR__, 4) . '/cli/batchgapfix.php');
expect($poller)->not->toBeFalse('lib/poller.php must be readable')
->and($dsstats)->not->toBeFalse('lib/dsstats.php must be readable')
->and($batchgapfix)->not->toBeFalse('cli/batchgapfix.php must be readable');
// the bare existence test is what the fix removed; a reappearance means a
// call site went back to trusting a recycled pid
expect($poller)->not->toContain("posix_kill(\$r['pid'], 0)")
->and($dsstats)->not->toContain("posix_kill(\$p['pid'], 0)")
->and($batchgapfix)->toContain("cacti_process_still_running((int) \$r['pid'])")
->and($batchgapfix)->not->toContain("posix_kill(\$r['pid'], 0)");
});
/**
* The registry holds CLI tasks. A web request asking whether one is alive has a
* different command name by definition, so comparing identities there would
* report a running poller dead and let a second copy start.
*/
test('the identity check is limited to CLI callers', function () {
$body = file_get_contents(dirname(__DIR__, 4) . '/lib/poller.php');
$start = strpos($body, 'function cacti_process_still_running(');
expect($start)->not->toBeFalse();
$function = substr($body, $start, strpos($body, "\nfunction ", $start + 1) - $start);
$sapi_guard = strpos($function, "PHP_SAPI !== 'cli'");
$proc_read = strpos($function, '/proc/');
expect($sapi_guard)->not->toBeFalse()
->and($sapi_guard)->toBeLessThan($proc_read);
});
/**
* The registry's children are spawned from path_php_binary, which is routinely a
* versioned path, while the parent may have been started as plain php. Both are
* the same interpreter but report different command names, so an identity check
* built on the name alone calls a live task dead and lets a second copy start.
*/
test('a child started through another name for our interpreter is still ours', function () {
if (!is_dir('/proc') || PHP_SAPI !== 'cli') {
test()->markTestSkipped('the identity check needs /proc and a CLI caller');
}
$alias = sys_get_temp_dir() . '/cacti_php_alias_' . bin2hex(random_bytes(6));
if (!@symlink(PHP_BINARY, $alias)) {
test()->markTestSkipped('cannot create a second name for the interpreter');
}
$pipes = [];
$handle = proc_open([$alias, '-r', 'sleep(30);'], [1 => ['pipe', 'w'], 2 => ['pipe', 'w']], $pipes);
$pid = (int) proc_get_status($handle)['pid'];
foreach ($pipes as $pipe) {
fclose($pipe);
}
for ($wait = 0; $wait < 50 && !is_readable('/proc/' . $pid . '/comm'); $wait++) {
usleep(20000);
}
$mine = trim((string) @file_get_contents('/proc/' . getmypid() . '/comm'));
$theirs = trim((string) @file_get_contents('/proc/' . $pid . '/comm'));
/* Whether a second path to the interpreter produces a second command name is
the runner's business, not ours. Where php is already the plain name both
sides read alike and there is no mismatch left to demonstrate, so skip
rather than assert an environment detail. */
if ($theirs === $mine) {
proc_terminate($handle);
proc_close($handle);
unlink($alias);
test()->markTestSkipped('this interpreter reports one command name for both paths');
}
// the names differ here, so the check this replaced called the child a stranger
expect(cacti_process_still_running($pid))->toBeTrue();
proc_terminate($handle);
proc_close($handle);
unlink($alias);
});