From 0b796fb555f68baa30c690b5c1c7d454fa8541d5 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 18:55:41 -0700 Subject: [PATCH 01/10] fix: restore the advertised PHP 7.4 runtime floor Signed-off-by: Thomas Vincent --- .github/workflows/code-quality.yml | 19 ++++++++++ README.md | 5 +++ audit_functions.php | 22 ++++++------ audit_syslog.php | 32 ++++++++--------- setup.php | 2 +- tests/Security/Php74CompatibilityTest.php | 34 ++++++++++++++++++ tests/Security/Php81SyntaxTest.php | 43 ----------------------- 7 files changed, 86 insertions(+), 71 deletions(-) create mode 100644 tests/Security/Php74CompatibilityTest.php delete mode 100644 tests/Security/Php81SyntaxTest.php diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 0e9994a..fcbd69d 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -32,6 +32,25 @@ on: - develop jobs: + php74-runtime-syntax: + name: PHP 7.4 advertised runtime syntax + runs-on: ubuntu-latest + + steps: + - name: Checkout audit Plugin + uses: actions/checkout@v4 + + - name: Install PHP 7.4 + uses: shivammathur/setup-php@v2 + with: + php-version: '7.4' + coverage: none + + - name: Lint runtime PHP + run: | + find . -path './.git' -prune -o -path './phpstan' -prune -o -path './tests' -prune \ + -o -name '*.php' -print0 | xargs -0 -n1 php -l + code-quality: runs-on: ubuntu-latest diff --git a/README.md b/README.md index cbffe64..b900f16 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,10 @@ # audit +The plugin supports Cacti 1.2.20 and later and keeps its runtime files compatible +with PHP 7.4. Development analysis and the full security suite run on current PHP +versions; CI separately parses every production PHP file with PHP 7.4 so the +advertised installation floor cannot drift unnoticed. + This plugin is to be used to track transactions in the Cacti database, when they were made, by what IP address and by what login account. This can be used to determine the root cause of issues created by users of the Cacti system. diff --git a/audit_functions.php b/audit_functions.php index db65fe9..045261f 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -9,7 +9,7 @@ function audit_user_is_admin(): bool { /** * @param array $selected_items */ -function audit_process_page_data(string $page, mixed $drop_action, array $selected_items): string { +function audit_process_page_data(string $page, $drop_action, array $selected_items): string { $objects = []; if ($drop_action !== false) { @@ -136,11 +136,11 @@ function audit_process_page_data(string $page, mixed $drop_action, array $select return audit_json_encode($objects); } -function audit_is_sensitive_key(mixed $key): int|false { +function audit_is_sensitive_key($key) { return preg_match('/(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', (string) $key); } -function audit_redact_sensitive_data(mixed $data): mixed { +function audit_redact_sensitive_data($data) { if (!is_array($data)) { return $data; } @@ -160,7 +160,7 @@ function audit_redact_sensitive_data(mixed $data): mixed { return $redacted; } -function audit_redact_sensitive_value(mixed $value): mixed { +function audit_redact_sensitive_value($value) { if (!is_string($value)) { return $value; } @@ -174,7 +174,7 @@ function audit_redact_sensitive_value(mixed $value): mixed { return preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $value); } -function audit_bound_log_data(mixed $data, int $depth = 0, ?object $state = null): mixed { +function audit_bound_log_data($data, int $depth = 0, ?object $state = null) { if ($state === null) { $state = (object) ['fields' => 0]; } @@ -243,7 +243,7 @@ function audit_redact_cli_arguments(array $arguments): array { return $redacted; } -function audit_json_encode(mixed $data, int $options = 0): string { +function audit_json_encode($data, int $options = 0): string { $json = json_encode(audit_bound_log_data($data), JSON_INVALID_UTF8_SUBSTITUTE | $options, 16); if ($json === false) { @@ -255,7 +255,7 @@ function audit_json_encode(mixed $data, int $options = 0): string { return $json; } -function audit_json_decode(mixed $json, ?string &$error = null): mixed { +function audit_json_decode($json, ?string &$error = null) { $error = null; try { @@ -320,7 +320,7 @@ function audit_event_integrity_hash(array $event): string { return hash('sha256', audit_json_encode($material, JSON_UNESCAPED_SLASHES)); } -function audit_event_type_for_request(mixed $page, mixed $action): string { +function audit_event_type_for_request($page, $action): string { $page_name = preg_replace('/\.php$/', '', (string) $page); $page_name = preg_replace('/[^a-z0-9_]+/i', '_', $page_name ?? ''); $verb = preg_replace('/[^a-z0-9_]+/i', '_', strtolower((string) $action)); @@ -392,7 +392,7 @@ function audit_external_log_format(array $data, string $format = 'json'): string return audit_json_encode($data, JSON_UNESCAPED_SLASHES) . "\n"; } -function audit_csv_safe_cell(mixed $value): string { +function audit_csv_safe_cell($value): string { $value = (string) $value; if (preg_match('/^[=+\-@]/', ltrim($value))) { @@ -402,7 +402,7 @@ function audit_csv_safe_cell(mixed $value): string { return $value; } -function audit_retention_cutoff(mixed $retention, ?DateTimeImmutable $now = null): DateTimeImmutable { +function audit_retention_cutoff($retention, ?DateTimeImmutable $now = null): DateTimeImmutable { $now = $now instanceof DateTimeImmutable ? $now->setTimezone(new DateTimeZone('UTC')) : new DateTimeImmutable('now', new DateTimeZone('UTC')); @@ -562,7 +562,7 @@ function audit_operation_verifier_for_request(string $page, array $post): ?array /** * @return array */ -function audit_verify_operation(mixed $verifier): array { +function audit_verify_operation($verifier): array { if (!is_array($verifier) || empty($verifier['type'])) { return ['outcome' => 'unknown', 'reason' => null]; } diff --git a/audit_syslog.php b/audit_syslog.php index 10ff4f0..b3e4d37 100644 --- a/audit_syslog.php +++ b/audit_syslog.php @@ -19,7 +19,7 @@ function audit_syslog_enabled(): bool { return read_config_option('audit_syslog_enabled') == 'on'; } -function audit_syslog_read_setting(string $name, mixed $default): mixed { +function audit_syslog_read_setting(string $name, $default) { $value = read_config_option($name); return $value === '' || $value === null ? $default : $value; @@ -28,7 +28,7 @@ function audit_syslog_read_setting(string $name, mixed $default): mixed { /** * @param array $errors */ -function audit_syslog_bounded_integer(mixed $value, int $default, int $minimum, int $maximum, array &$errors, string $name): int { +function audit_syslog_bounded_integer($value, int $default, int $minimum, int $maximum, array &$errors, string $name): int { if (!is_scalar($value) || !preg_match('/^[0-9]+$/', (string) $value)) { $errors[] = $name . '_invalid'; @@ -268,7 +268,7 @@ function audit_syslog_facilities(): array { ]; } -function audit_syslog_severity_code(mixed $severity): int { +function audit_syslog_severity_code($severity): int { $map = [ 'emergency' => 0, 'emerg' => 0, 'alert' => 1, 'critical' => 2, 'crit' => 2, 'error' => 3, 'err' => 3, 'warning' => 4, @@ -279,20 +279,20 @@ function audit_syslog_severity_code(mixed $severity): int { return isset($map[$severity]) ? $map[$severity] : 6; } -function audit_syslog_header_token(mixed $value, int $maximum, string $fallback): string { +function audit_syslog_header_token($value, int $maximum, string $fallback): string { $value = preg_replace('/[^\\x21-\\x3c\\x3e-\\x5a\\x5e-\\x7e]/', '_', (string) $value); $value = substr($value ?? '', 0, $maximum); return $value === '' ? $fallback : $value; } -function audit_syslog_structured_value(mixed $value): string { +function audit_syslog_structured_value($value): string { $value = preg_replace('/[\\x00-\\x1f\\x7f]/', ' ', (string) $value); return str_replace(['\\', '"', ']'], ['\\\\', '\\"', '\\]'], $value ?? ''); } -function audit_syslog_timestamp(mixed $value): string { +function audit_syslog_timestamp($value): string { $value = (string) $value; if (preg_match('/^([0-9]{4}-[0-9]{2}-[0-9]{2})[ T]([0-9]{2}:[0-9]{2}:[0-9]{2})(\\.[0-9]{1,6})?/', $value, $matches)) { @@ -315,11 +315,11 @@ function audit_syslog_normalized_data(array $event, array $config): array { return $data; } -function audit_syslog_cef_escape_header(mixed $value): string { +function audit_syslog_cef_escape_header($value): string { return str_replace(['\\', '|', "\r", "\n"], ['\\\\', '\\|', ' ', ' '], (string) $value); } -function audit_syslog_cef_escape_extension(mixed $value): string { +function audit_syslog_cef_escape_extension($value): string { return str_replace( ['\\', '=', "\r", "\n"], ['\\\\', '\\=', '\\r', '\\n'], @@ -327,7 +327,7 @@ function audit_syslog_cef_escape_extension(mixed $value): string { ); } -function audit_syslog_cef_severity(mixed $severity): int { +function audit_syslog_cef_severity($severity): int { $map = [ 'emergency' => 10, 'emerg' => 10, 'alert' => 10, 'critical' => 9, 'crit' => 9, 'error' => 8, 'err' => 8, @@ -339,7 +339,7 @@ function audit_syslog_cef_severity(mixed $severity): int { return isset($map[$severity]) ? $map[$severity] : 3; } -function audit_syslog_cef_event_field(mixed $value): string { +function audit_syslog_cef_event_field($value): string { if (is_string($value) && $value !== '') { $decoded = audit_json_decode($value, $error); @@ -526,7 +526,7 @@ function audit_syslog_socket_target(array $config): string { return $scheme . '://' . $receiver . ':' . $config['port']; } -function audit_syslog_stream_operation(callable $operation, string &$warning = ''): mixed { +function audit_syslog_stream_operation(callable $operation, string &$warning = '') { $warning = ''; $handler = function ($severity, $message) use (&$warning) { $warning = audit_syslog_bounded_error($message); @@ -614,13 +614,13 @@ function audit_syslog_open_socket(array $config): array { return ['socket' => $socket, 'error_code' => '', 'error' => '']; } -function audit_syslog_bounded_error(mixed $error): string { +function audit_syslog_bounded_error($error): string { $error = preg_replace('/[\\x00-\\x1f\\x7f]+/', ' ', (string) $error); return substr(trim($error ?? ''), 0, 1024); } -function audit_syslog_fwrite(mixed $socket, string $message, string &$warning = ''): int|false { +function audit_syslog_fwrite($socket, string $message, string &$warning = '') { return audit_syslog_stream_operation(function () use ($socket, $message) { return fwrite($socket, $message); }, $warning); @@ -629,7 +629,7 @@ function audit_syslog_fwrite(mixed $socket, string $message, string &$warning = /** * @return array */ -function audit_syslog_write(mixed $socket, string $message, string $transport): array { +function audit_syslog_write($socket, string $message, string $transport): array { if (!is_resource($socket)) { return ['status' => 'failed', 'error_code' => 'socket_unavailable', 'error' => 'Syslog socket is unavailable.']; } @@ -674,7 +674,7 @@ function audit_syslog_write(mixed $socket, string $message, string $transport): * @param array $config * @return array */ -function audit_syslog_send_event(array $event, array $config, mixed &$socket = null): array { +function audit_syslog_send_event(array $event, array $config, &$socket = null): array { $formatted = audit_syslog_record($event, $config); if ($formatted['status'] !== 'ready') { @@ -758,7 +758,7 @@ function audit_syslog_delivery_config(array $config, array $delivery): array { /** * @param array $config */ -function audit_syslog_retry_delay(mixed $attempt, array $config): int { +function audit_syslog_retry_delay($attempt, array $config): int { $exponent = min(max(0, (int) $attempt - 1), 30); $delay = $config['retry_base'] * pow(2, $exponent); diff --git a/setup.php b/setup.php index 41dadc8..b86119f 100644 --- a/setup.php +++ b/setup.php @@ -368,7 +368,7 @@ function audit_setup_syslog_table(): void { AFTER node_id'); } -function audit_upgrade_event_schema(mixed $rcnn_id = false): void { +function audit_upgrade_event_schema($rcnn_id = false): void { $remote = $rcnn_id !== false; $args = $remote ? [true, $rcnn_id] : []; $columns = [ diff --git a/tests/Security/Php74CompatibilityTest.php b/tests/Security/Php74CompatibilityTest.php new file mode 100644 index 0000000..08db874 --- /dev/null +++ b/tests/Security/Php74CompatibilityTest.php @@ -0,0 +1,34 @@ +not->toBeFalse() + ->and($contents)->not->toMatch('/\bmixed\s+[&]?\$/') + ->and($contents)->not->toMatch('/:\s*(?:mixed|[^\s{]+\|[^\s{]+)/'); + } + }); + + it('keeps the compatibility floor explicit in plugin metadata', function () { + $info = parse_ini_file(__DIR__ . '/../../INFO', true); + + expect($info['info']['compat'] ?? null)->toBe('1.2.20'); + }); +}); diff --git a/tests/Security/Php81SyntaxTest.php b/tests/Security/Php81SyntaxTest.php deleted file mode 100644 index f0daa1f..0000000 --- a/tests/Security/Php81SyntaxTest.php +++ /dev/null @@ -1,43 +0,0 @@ -not->toBeFalse("Required plugin file is missing: {$relativeFile}"); - expect(is_readable($path))->toBeTrue("Required plugin file is unreadable: {$relativeFile}"); - } - }); - - it('uses short array syntax', function () use ($files) { - foreach ($files as $relativeFile) { - $path = realpath(__DIR__ . '/../../' . $relativeFile); - $contents = file_get_contents($path); - - expect(preg_match('/\barray\s*\(/', $contents))->toBe(0, - "{$relativeFile} still uses long array() syntax" - ); - } - }); -}); From 87ddc3ae8db5aff2557967e7c09c4386bb59f4e2 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 19:30:00 -0700 Subject: [PATCH 02/10] ci: restore audit quality and integration jobs --- .github/workflows/plugin-ci-workflow.yml | 2 +- audit_functions.php | 12 +++++++----- audit_syslog.php | 10 +++++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index befe032..219e341 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -43,7 +43,7 @@ jobs: services: mysql: - image: mysql:8.0 + image: mariadb:10.11.18 env: MYSQL_ROOT_PASSWORD: cactiroot MYSQL_DATABASE: cacti diff --git a/audit_functions.php b/audit_functions.php index 045261f..488f62d 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -8,6 +8,7 @@ function audit_user_is_admin(): bool { /** * @param array $selected_items + * @param mixed $drop_action */ function audit_process_page_data(string $page, $drop_action, array $selected_items): string { $objects = []; @@ -136,11 +137,11 @@ function audit_process_page_data(string $page, $drop_action, array $selected_ite return audit_json_encode($objects); } -function audit_is_sensitive_key($key) { +function audit_is_sensitive_key($key) { return preg_match('/(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', (string) $key); } -function audit_redact_sensitive_data($data) { +function audit_redact_sensitive_data($data) { if (!is_array($data)) { return $data; } @@ -160,7 +161,7 @@ function audit_redact_sensitive_data($data) { return $redacted; } -function audit_redact_sensitive_value($value) { +function audit_redact_sensitive_value($value) { if (!is_string($value)) { return $value; } @@ -174,7 +175,7 @@ function audit_redact_sensitive_value($value) { return preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $value); } -function audit_bound_log_data($data, int $depth = 0, ?object $state = null) { +function audit_bound_log_data($data, int $depth = 0, ?object $state = null) { if ($state === null) { $state = (object) ['fields' => 0]; } @@ -255,7 +256,7 @@ function audit_json_encode($data, int $options = 0): string { return $json; } -function audit_json_decode($json, ?string &$error = null) { +function audit_json_decode($json, ?string &$error = null) { $error = null; try { @@ -561,6 +562,7 @@ function audit_operation_verifier_for_request(string $page, array $post): ?array /** * @return array + * @param mixed $verifier */ function audit_verify_operation($verifier): array { if (!is_array($verifier) || empty($verifier['type'])) { diff --git a/audit_syslog.php b/audit_syslog.php index b3e4d37..0933a38 100644 --- a/audit_syslog.php +++ b/audit_syslog.php @@ -19,7 +19,7 @@ function audit_syslog_enabled(): bool { return read_config_option('audit_syslog_enabled') == 'on'; } -function audit_syslog_read_setting(string $name, $default) { +function audit_syslog_read_setting(string $name, $default) { $value = read_config_option($name); return $value === '' || $value === null ? $default : $value; @@ -27,6 +27,7 @@ function audit_syslog_read_setting(string $name, $default) { /** * @param array $errors + * @param mixed $value */ function audit_syslog_bounded_integer($value, int $default, int $minimum, int $maximum, array &$errors, string $name): int { if (!is_scalar($value) || !preg_match('/^[0-9]+$/', (string) $value)) { @@ -526,7 +527,7 @@ function audit_syslog_socket_target(array $config): string { return $scheme . '://' . $receiver . ':' . $config['port']; } -function audit_syslog_stream_operation(callable $operation, string &$warning = '') { +function audit_syslog_stream_operation(callable $operation, string &$warning = '') { $warning = ''; $handler = function ($severity, $message) use (&$warning) { $warning = audit_syslog_bounded_error($message); @@ -620,7 +621,7 @@ function audit_syslog_bounded_error($error): string { return substr(trim($error ?? ''), 0, 1024); } -function audit_syslog_fwrite($socket, string $message, string &$warning = '') { +function audit_syslog_fwrite($socket, string $message, string &$warning = '') { return audit_syslog_stream_operation(function () use ($socket, $message) { return fwrite($socket, $message); }, $warning); @@ -628,6 +629,7 @@ function audit_syslog_fwrite($socket, string $message, string &$warning = '') { /** * @return array + * @param mixed $socket */ function audit_syslog_write($socket, string $message, string $transport): array { if (!is_resource($socket)) { @@ -672,6 +674,7 @@ function audit_syslog_write($socket, string $message, string $transport): array /** * @param array $event * @param array $config + * @param null|mixed $socket * @return array */ function audit_syslog_send_event(array $event, array $config, &$socket = null): array { @@ -757,6 +760,7 @@ function audit_syslog_delivery_config(array $config, array $delivery): array { /** * @param array $config + * @param mixed $attempt */ function audit_syslog_retry_delay($attempt, array $config): int { $exponent = min(max(0, (int) $attempt - 1), 30); From e05a49964fba84ed8fe1be91091a17e8a38cd69c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 19:40:53 -0700 Subject: [PATCH 03/10] fix: preserve PHP 7.4 runtime semantics --- audit.php | 9 +++-- audit_functions.php | 37 +++++++++++++++++++-- audit_syslog.php | 40 ++++++++++++++++++++++- setup.php | 3 ++ tests/Security/Php74CompatibilityTest.php | 37 +++++++++++++++++++-- tests/controller_security_test.php | 10 ++++++ tests/syslog_queue_test.php | 19 ++++++++--- 7 files changed, 141 insertions(+), 14 deletions(-) diff --git a/audit.php b/audit.php index dcbf213..99e8762 100644 --- a/audit.php +++ b/audit.php @@ -128,7 +128,7 @@ WHERE id = ?', [get_filter_request_var('id')]); - if (!is_array($data)) { + if (!is_array($data) || !cacti_sizeof($data)) { http_response_code(404); print html_escape(__('Audit event not found.', 'audit')); @@ -185,7 +185,7 @@ function audit_render_event_details(array $data): string { LIMIT 1', [$data['id']]); - if (is_array($syslog)) { + if (is_array($syslog) && cacti_sizeof($syslog)) { $output .= '
' . __('Remote Syslog Delivery:', 'audit') . ' ' . html_escape($syslog['state']) . ''; $output .= '
' . __('Syslog Attempts:', 'audit') . ' ' . (int) $syslog['attempts'] . ''; @@ -251,7 +251,10 @@ function audit_render_event_details(array $data): string { return $output . ''; } -function audit_render_value(mixed $value): string { +/** + * @param mixed $value + */ +function audit_render_value($value): string { if (is_array($value) || is_object($value)) { return '
' . html_escape(json_encode($value, JSON_PRETTY_PRINT | JSON_INVALID_UTF8_SUBSTITUTE)) . '
'; } diff --git a/audit_functions.php b/audit_functions.php index 488f62d..8405899 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -137,10 +137,18 @@ function audit_process_page_data(string $page, $drop_action, array $selected_ite return audit_json_encode($objects); } +/** + * @param mixed $key + * @return int|false + */ function audit_is_sensitive_key($key) { return preg_match('/(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', (string) $key); } +/** + * @param mixed $data + * @return mixed + */ function audit_redact_sensitive_data($data) { if (!is_array($data)) { return $data; @@ -161,6 +169,10 @@ function audit_redact_sensitive_data($data) { return $redacted; } +/** + * @param mixed $value + * @return mixed + */ function audit_redact_sensitive_value($value) { if (!is_string($value)) { return $value; @@ -175,6 +187,10 @@ function audit_redact_sensitive_value($value) { return preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $value); } +/** + * @param mixed $data + * @return mixed + */ function audit_bound_log_data($data, int $depth = 0, ?object $state = null) { if ($state === null) { $state = (object) ['fields' => 0]; @@ -244,6 +260,9 @@ function audit_redact_cli_arguments(array $arguments): array { return $redacted; } +/** + * @param mixed $data + */ function audit_json_encode($data, int $options = 0): string { $json = json_encode(audit_bound_log_data($data), JSON_INVALID_UTF8_SUBSTITUTE | $options, 16); @@ -256,6 +275,10 @@ function audit_json_encode($data, int $options = 0): string { return $json; } +/** + * @param mixed $json + * @return mixed + */ function audit_json_decode($json, ?string &$error = null) { $error = null; @@ -321,6 +344,10 @@ function audit_event_integrity_hash(array $event): string { return hash('sha256', audit_json_encode($material, JSON_UNESCAPED_SLASHES)); } +/** + * @param mixed $page + * @param mixed $action + */ function audit_event_type_for_request($page, $action): string { $page_name = preg_replace('/\.php$/', '', (string) $page); $page_name = preg_replace('/[^a-z0-9_]+/i', '_', $page_name ?? ''); @@ -393,6 +420,9 @@ function audit_external_log_format(array $data, string $format = 'json'): string return audit_json_encode($data, JSON_UNESCAPED_SLASHES) . "\n"; } +/** + * @param mixed $value + */ function audit_csv_safe_cell($value): string { $value = (string) $value; @@ -403,6 +433,9 @@ function audit_csv_safe_cell($value): string { return $value; } +/** + * @param mixed $retention + */ function audit_retention_cutoff($retention, ?DateTimeImmutable $now = null): DateTimeImmutable { $now = $now instanceof DateTimeImmutable ? $now->setTimezone(new DateTimeZone('UTC')) @@ -654,7 +687,7 @@ function audit_finalize_request(int $id, ?float $started_at = null, ?array $veri $event = db_fetch_row_prepared('SELECT * FROM audit_log WHERE id = ?', [$id]); - if (is_array($event)) { + if (is_array($event) && cacti_sizeof($event)) { db_execute_prepared('UPDATE audit_log SET integrity_hash = ? WHERE id = ?', [audit_event_integrity_hash($event), $id]); } @@ -705,7 +738,7 @@ function audit_record_event(string $event_type, array $options = []): int { $id = db_fetch_insert_id(); $event = db_fetch_row_prepared('SELECT * FROM audit_log WHERE id = ?', [$id]); - if (is_array($event)) { + if (is_array($event) && cacti_sizeof($event)) { db_execute_prepared('UPDATE audit_log SET integrity_hash = ? WHERE id = ?', [audit_event_integrity_hash($event), $id]); } diff --git a/audit_syslog.php b/audit_syslog.php index 0933a38..3eb94c5 100644 --- a/audit_syslog.php +++ b/audit_syslog.php @@ -19,6 +19,10 @@ function audit_syslog_enabled(): bool { return read_config_option('audit_syslog_enabled') == 'on'; } +/** + * @param mixed $default + * @return mixed + */ function audit_syslog_read_setting(string $name, $default) { $value = read_config_option($name); @@ -269,6 +273,9 @@ function audit_syslog_facilities(): array { ]; } +/** + * @param mixed $severity + */ function audit_syslog_severity_code($severity): int { $map = [ 'emergency' => 0, 'emerg' => 0, 'alert' => 1, 'critical' => 2, @@ -280,6 +287,9 @@ function audit_syslog_severity_code($severity): int { return isset($map[$severity]) ? $map[$severity] : 6; } +/** + * @param mixed $value + */ function audit_syslog_header_token($value, int $maximum, string $fallback): string { $value = preg_replace('/[^\\x21-\\x3c\\x3e-\\x5a\\x5e-\\x7e]/', '_', (string) $value); $value = substr($value ?? '', 0, $maximum); @@ -287,12 +297,18 @@ function audit_syslog_header_token($value, int $maximum, string $fallback): stri return $value === '' ? $fallback : $value; } +/** + * @param mixed $value + */ function audit_syslog_structured_value($value): string { $value = preg_replace('/[\\x00-\\x1f\\x7f]/', ' ', (string) $value); return str_replace(['\\', '"', ']'], ['\\\\', '\\"', '\\]'], $value ?? ''); } +/** + * @param mixed $value + */ function audit_syslog_timestamp($value): string { $value = (string) $value; @@ -316,10 +332,16 @@ function audit_syslog_normalized_data(array $event, array $config): array { return $data; } +/** + * @param mixed $value + */ function audit_syslog_cef_escape_header($value): string { return str_replace(['\\', '|', "\r", "\n"], ['\\\\', '\\|', ' ', ' '], (string) $value); } +/** + * @param mixed $value + */ function audit_syslog_cef_escape_extension($value): string { return str_replace( ['\\', '=', "\r", "\n"], @@ -328,6 +350,9 @@ function audit_syslog_cef_escape_extension($value): string { ); } +/** + * @param mixed $severity + */ function audit_syslog_cef_severity($severity): int { $map = [ 'emergency' => 10, 'emerg' => 10, 'alert' => 10, @@ -340,6 +365,9 @@ function audit_syslog_cef_severity($severity): int { return isset($map[$severity]) ? $map[$severity] : 3; } +/** + * @param mixed $value + */ function audit_syslog_cef_event_field($value): string { if (is_string($value) && $value !== '') { $decoded = audit_json_decode($value, $error); @@ -527,6 +555,9 @@ function audit_syslog_socket_target(array $config): string { return $scheme . '://' . $receiver . ':' . $config['port']; } +/** + * @return mixed + */ function audit_syslog_stream_operation(callable $operation, string &$warning = '') { $warning = ''; $handler = function ($severity, $message) use (&$warning) { @@ -615,12 +646,19 @@ function audit_syslog_open_socket(array $config): array { return ['socket' => $socket, 'error_code' => '', 'error' => '']; } +/** + * @param mixed $error + */ function audit_syslog_bounded_error($error): string { $error = preg_replace('/[\\x00-\\x1f\\x7f]+/', ' ', (string) $error); return substr(trim($error ?? ''), 0, 1024); } +/** + * @param mixed $socket + * @return int|false + */ function audit_syslog_fwrite($socket, string $message, string &$warning = '') { return audit_syslog_stream_operation(function () use ($socket, $message) { return fwrite($socket, $message); @@ -721,7 +759,7 @@ function audit_enqueue_syslog_event(int $audit_id): void { WHERE id = ?', [$audit_id]); - if (!is_array($event) || $event['request_status'] === 'started' || $event['event_uuid'] === '') { + if (!is_array($event) || !cacti_sizeof($event) || $event['request_status'] === 'started' || $event['event_uuid'] === '') { return; } diff --git a/setup.php b/setup.php index b86119f..4b3d3a8 100644 --- a/setup.php +++ b/setup.php @@ -368,6 +368,9 @@ function audit_setup_syslog_table(): void { AFTER node_id'); } +/** + * @param mixed $rcnn_id + */ function audit_upgrade_event_schema($rcnn_id = false): void { $remote = $rcnn_id !== false; $args = $remote ? [true, $rcnn_id] : []; diff --git a/tests/Security/Php74CompatibilityTest.php b/tests/Security/Php74CompatibilityTest.php index 08db874..a102174 100644 --- a/tests/Security/Php74CompatibilityTest.php +++ b/tests/Security/Php74CompatibilityTest.php @@ -15,17 +15,48 @@ 'index.php', 'setup.php', ]; + $hasPhp8OnlyType = static function (string $contents): bool { + $withoutComments = preg_replace('#/\*.*?\*/|//[^\r\n]*#s', '', $contents); - it('keeps PHP 8-only native types out of runtime files', function () use ($runtimeFiles) { + if ($withoutComments === null) { + return true; + } + + $namedType = '[?\\\\A-Za-z_][\\\\A-Za-z0-9_]*'; + $patterns = [ + '/\bfunction\s+[A-Za-z_]\w*\s*\([^)]*\bmixed\s+(?:&\s*)?\$[A-Za-z_]\w*/s', + '/\bfunction\s+[A-Za-z_]\w*\s*\([^)]*' . $namedType . '(?:\|' . $namedType . ')+\s+(?:&\s*)?\$[A-Za-z_]\w*/s', + '/\)\s*:\s*(?:mixed\b|' . $namedType . '(?:\|' . $namedType . ')+)/s', + ]; + + foreach ($patterns as $pattern) { + if (preg_match($pattern, $withoutComments) === 1) { + return true; + } + } + + return false; + }; + + it('keeps PHP 8-only syntax out of runtime files', function () use ($runtimeFiles, $hasPhp8OnlyType) { foreach ($runtimeFiles as $relativeFile) { $contents = file_get_contents(__DIR__ . '/../../' . $relativeFile); expect($contents)->not->toBeFalse() - ->and($contents)->not->toMatch('/\bmixed\s+[&]?\$/') - ->and($contents)->not->toMatch('/:\s*(?:mixed|[^\s{]+\|[^\s{]+)/'); + ->and($hasPhp8OnlyType((string) $contents))->toBeFalse() + ->and($contents)->not->toContain('str_contains(') + ->and($contents)->not->toContain('str_starts_with(') + ->and($contents)->not->toContain('str_ends_with(') + ->and($contents)->not->toContain('?->'); } }); + it('distinguishes PHPDoc and regex text from native declarations', function () use ($hasPhp8OnlyType) { + expect($hasPhp8OnlyType("/** @param mixed \$value */\nfunction safe(\$value) { return '/a|b/'; }"))->toBeFalse() + ->and($hasPhp8OnlyType('function nativeMixed(mixed $value) {}'))->toBeTrue() + ->and($hasPhp8OnlyType('function nativeUnion($value): int|false {}'))->toBeTrue(); + }); + it('keeps the compatibility floor explicit in plugin metadata', function () { $info = parse_ini_file(__DIR__ . '/../../INFO', true); diff --git a/tests/controller_security_test.php b/tests/controller_security_test.php index 80ba0c0..94d41b7 100644 --- a/tests/controller_security_test.php +++ b/tests/controller_security_test.php @@ -90,6 +90,16 @@ exit(1); } +$missing_event_guard = strpos((string) $controller, 'if (!is_array($data) || !cacti_sizeof($data))'); +$missing_event_404 = strpos((string) $controller, 'http_response_code(404)', (int) $missing_event_guard); +$view_event_write = strpos((string) $controller, "audit_record_event('audit.event.viewed'"); + +if ($missing_event_guard === false || $missing_event_404 === false || $view_event_write === false || + $missing_event_guard > $missing_event_404 || $missing_event_404 > $view_event_write) { + fwrite(STDERR, 'Missing audit events must return before recording a view event.' . PHP_EOL); + exit(1); +} + if (strpos($functions, 'audit_enforce_syslog_settings_request()') === false || strpos($functions, "'audit.syslog.configuration.denied'") === false) { fwrite(STDERR, 'Remote Syslog settings must enforce Audit Log Admin on save.' . PHP_EOL); diff --git a/tests/syslog_queue_test.php b/tests/syslog_queue_test.php index b88913c..d535cf5 100644 --- a/tests/syslog_queue_test.php +++ b/tests/syslog_queue_test.php @@ -23,6 +23,11 @@ ]; $audit_queue_calls = []; $audit_queue_affected_rows = 0; +$audit_queue_event = [ + 'id' => 42, + 'event_uuid' => '32e0a97d-d9e8-4abc-8f41-2bbbc50793ca', + 'request_status' => 'completed' +]; function read_config_option($name) { global $audit_queue_settings; @@ -35,11 +40,9 @@ function db_table_exists($table) { } function db_fetch_row_prepared($sql, $params = []) { - return [ - 'id' => (int) $params[0], - 'event_uuid' => '32e0a97d-d9e8-4abc-8f41-2bbbc50793ca', - 'request_status' => 'completed' - ]; + global $audit_queue_event; + + return $audit_queue_event; } function db_execute_prepared($sql, $params = []) { @@ -89,6 +92,12 @@ function audit_queue_assert($condition, $message) { audit_queue_assert($audit_queue_calls[0]['params'][5] === 'pending', 'A valid enabled destination must enqueue in pending state.'); +$audit_queue_calls = []; +$audit_queue_event = []; +audit_enqueue_syslog_event(404); +audit_queue_assert($audit_queue_calls === [], + 'A missing audit row must not enqueue a delivery with a null audit ID.'); + $config = audit_syslog_config(); $retry_identity = audit_syslog_delivery_config($config, [ 'delivery_node_id' => 'original-node', From 99f5e651b18f038cd5b76796d06f2bc097eadbf4 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 19:52:13 -0700 Subject: [PATCH 04/10] style: use canonical Cacti row guards --- audit.php | 6 ++++-- audit_functions.php | 6 ++++-- audit_syslog.php | 7 ++++++- tests/controller_security_test.php | 2 +- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/audit.php b/audit.php index 99e8762..283b381 100644 --- a/audit.php +++ b/audit.php @@ -128,12 +128,13 @@ WHERE id = ?', [get_filter_request_var('id')]); - if (!is_array($data) || !cacti_sizeof($data)) { + if (!cacti_sizeof($data)) { http_response_code(404); print html_escape(__('Audit event not found.', 'audit')); break; } + /** @var array $data */ audit_record_event('audit.event.viewed', [ 'event_category' => 'audit', @@ -185,7 +186,8 @@ function audit_render_event_details(array $data): string { LIMIT 1', [$data['id']]); - if (is_array($syslog) && cacti_sizeof($syslog)) { + if (cacti_sizeof($syslog)) { + /** @var array $syslog */ $output .= '
' . __('Remote Syslog Delivery:', 'audit') . ' ' . html_escape($syslog['state']) . ''; $output .= '
' . __('Syslog Attempts:', 'audit') . ' ' . (int) $syslog['attempts'] . ''; diff --git a/audit_functions.php b/audit_functions.php index 8405899..2b27a30 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -687,7 +687,8 @@ function audit_finalize_request(int $id, ?float $started_at = null, ?array $veri $event = db_fetch_row_prepared('SELECT * FROM audit_log WHERE id = ?', [$id]); - if (is_array($event) && cacti_sizeof($event)) { + if (cacti_sizeof($event)) { + /** @var array $event */ db_execute_prepared('UPDATE audit_log SET integrity_hash = ? WHERE id = ?', [audit_event_integrity_hash($event), $id]); } @@ -738,7 +739,8 @@ function audit_record_event(string $event_type, array $options = []): int { $id = db_fetch_insert_id(); $event = db_fetch_row_prepared('SELECT * FROM audit_log WHERE id = ?', [$id]); - if (is_array($event) && cacti_sizeof($event)) { + if (cacti_sizeof($event)) { + /** @var array $event */ db_execute_prepared('UPDATE audit_log SET integrity_hash = ? WHERE id = ?', [audit_event_integrity_hash($event), $id]); } diff --git a/audit_syslog.php b/audit_syslog.php index 3eb94c5..5de9ffa 100644 --- a/audit_syslog.php +++ b/audit_syslog.php @@ -759,7 +759,12 @@ function audit_enqueue_syslog_event(int $audit_id): void { WHERE id = ?', [$audit_id]); - if (!is_array($event) || !cacti_sizeof($event) || $event['request_status'] === 'started' || $event['event_uuid'] === '') { + if (!cacti_sizeof($event)) { + return; + } + /** @var array $event */ + + if ($event['request_status'] === 'started' || $event['event_uuid'] === '') { return; } diff --git a/tests/controller_security_test.php b/tests/controller_security_test.php index 94d41b7..de4b740 100644 --- a/tests/controller_security_test.php +++ b/tests/controller_security_test.php @@ -90,7 +90,7 @@ exit(1); } -$missing_event_guard = strpos((string) $controller, 'if (!is_array($data) || !cacti_sizeof($data))'); +$missing_event_guard = strpos((string) $controller, 'if (!cacti_sizeof($data))'); $missing_event_404 = strpos((string) $controller, 'http_response_code(404)', (int) $missing_event_guard); $view_event_write = strpos((string) $controller, "audit_record_event('audit.event.viewed'"); From 786ecd0b4cd6d442db66b444ad03092dcfbd2f14 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 20:05:28 -0700 Subject: [PATCH 05/10] fix: fail closed when audit redaction fails --- audit.php | 123 +++++++++++++++--------------- audit_functions.php | 4 +- tests/security_functions_test.php | 11 +++ 3 files changed, 74 insertions(+), 64 deletions(-) diff --git a/audit.php b/audit.php index 283b381..e4d848a 100644 --- a/audit.php +++ b/audit.php @@ -353,6 +353,7 @@ function audit_export_rows(): void { ]); if (cacti_sizeof($events)) { + /** @var array> $events */ header('Content-Disposition: attachment; filename=audit_export.csv'); header('Content-Type: text/csv; charset=UTF-8'); header('X-Content-Type-Options: nosniff'); @@ -362,44 +363,42 @@ function audit_export_rows(): void { if ($output !== false) { fputcsv($output, ['event_uuid', 'correlation_id', 'event_type', 'event_category', 'severity', 'page', 'user_id', 'username', 'action', 'request_status', 'operation_outcome', 'outcome_reason', 'target_type', 'target_id', 'external_status', 'external_error', 'ip_address', 'user_agent', 'http_method', 'http_status', 'event_time', 'completed_time', 'duration_ms', 'integrity_hash', 'post', 'details'], ',', '"', ''); - if (is_array($events)) { - foreach ($events as $event) { - if ($event['action'] == 'cli') { - $poster = $event['post']; - } else { - $post = audit_json_decode($event['post'], $json_error); - $poster = is_array($post) ? json_encode($post, JSON_INVALID_UTF8_SUBSTITUTE) : $event['post']; - } - - fputcsv($output, array_map('audit_csv_safe_cell', [ - $event['event_uuid'], - $event['correlation_id'], - $event['event_type'], - $event['event_category'], - $event['severity'], - $event['page'], - $event['user_id'], - get_username($event['user_id']), - $event['action'], - $event['request_status'], - $event['operation_outcome'], - $event['outcome_reason'], - $event['target_type'], - $event['target_id'], - $event['external_status'], - $event['external_error'], - $event['ip_address'], - $event['user_agent'], - $event['http_method'], - $event['http_status'], - $event['event_time'], - $event['completed_time'], - $event['duration_ms'], - $event['integrity_hash'], - $poster, - $event['details'] - ]), ',', '"', ''); + foreach ($events as $event) { + if ($event['action'] == 'cli') { + $poster = $event['post']; + } else { + $post = audit_json_decode($event['post'], $json_error); + $poster = is_array($post) ? json_encode($post, JSON_INVALID_UTF8_SUBSTITUTE) : $event['post']; } + + fputcsv($output, array_map('audit_csv_safe_cell', [ + $event['event_uuid'], + $event['correlation_id'], + $event['event_type'], + $event['event_category'], + $event['severity'], + $event['page'], + $event['user_id'], + get_username($event['user_id']), + $event['action'], + $event['request_status'], + $event['operation_outcome'], + $event['outcome_reason'], + $event['target_type'], + $event['target_id'], + $event['external_status'], + $event['external_error'], + $event['ip_address'], + $event['user_agent'], + $event['http_method'], + $event['http_status'], + $event['event_time'], + $event['completed_time'], + $event['duration_ms'], + $event['integrity_hash'], + $poster, + $event['details'] + ]), ',', '"', ''); } fclose($output); @@ -682,31 +681,31 @@ function audit_log(): void { $i = 0; if (cacti_sizeof($events)) { - if (is_array($events)) { - foreach ($events as $e) { - if ($e['action'] == 'cli') { - form_alternate_row('line' . $e['id'], false); - form_selectable_ecell($e['page'], $e['id']); - form_selectable_ecell($e['user_agent'], $e['id']); - form_selectable_cell('' . html_escape(ucfirst($e['action'])) . '', $e['id']); - form_selectable_ecell($e['request_status'], $e['id']); - form_selectable_ecell($e['external_status'], $e['id']); - form_selectable_cell(__('N/A', 'audit'), $e['id']); - form_selectable_ecell($e['ip_address'], $e['id'], '', 'right'); - form_selectable_ecell($e['event_time'], $e['id'], '', 'right'); - form_end_row(); - } else { - form_alternate_row('line' . $e['id'], false); - form_selectable_cell(filter_value($e['page'], get_request_var('filter')), $e['id']); - form_selectable_ecell($e['username'], $e['id']); - form_selectable_cell('' . html_escape(ucfirst($e['action'])) . '', $e['id']); - form_selectable_ecell($e['request_status'], $e['id']); - form_selectable_ecell($e['external_status'], $e['id']); - form_selectable_ecell($e['user_agent'], $e['id']); - form_selectable_ecell($e['ip_address'], $e['id'], '', 'right'); - form_selectable_ecell($e['event_time'], $e['id'], '', 'right'); - form_end_row(); - } + /** @var array> $events */ + + foreach ($events as $e) { + if ($e['action'] == 'cli') { + form_alternate_row('line' . $e['id'], false); + form_selectable_ecell($e['page'], $e['id']); + form_selectable_ecell($e['user_agent'], $e['id']); + form_selectable_cell('' . html_escape(ucfirst($e['action'])) . '', $e['id']); + form_selectable_ecell($e['request_status'], $e['id']); + form_selectable_ecell($e['external_status'], $e['id']); + form_selectable_cell(__('N/A', 'audit'), $e['id']); + form_selectable_ecell($e['ip_address'], $e['id'], '', 'right'); + form_selectable_ecell($e['event_time'], $e['id'], '', 'right'); + form_end_row(); + } else { + form_alternate_row('line' . $e['id'], false); + form_selectable_cell(filter_value($e['page'], get_request_var('filter')), $e['id']); + form_selectable_ecell($e['username'], $e['id']); + form_selectable_cell('' . html_escape(ucfirst($e['action'])) . '', $e['id']); + form_selectable_ecell($e['request_status'], $e['id']); + form_selectable_ecell($e['external_status'], $e['id']); + form_selectable_ecell($e['user_agent'], $e['id']); + form_selectable_ecell($e['ip_address'], $e['id'], '', 'right'); + form_selectable_ecell($e['event_time'], $e['id'], '', 'right'); + form_end_row(); } } } else { diff --git a/audit_functions.php b/audit_functions.php index 2b27a30..898bfae 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -254,7 +254,7 @@ function audit_redact_cli_arguments(array $arguments): array { continue; } - $redacted[] = preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $argument) ?? $argument; + $redacted[] = preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $argument) ?? '[REDACTED]'; } return $redacted; @@ -479,7 +479,7 @@ function audit_deliver_external_event(int $id): void { $event = db_fetch_row_prepared('SELECT * FROM audit_log WHERE id = ?', [$id]); - if (!is_array($event) || $event === [] || ($event['request_status'] ?? '') === 'started') { + if (!is_array($event) || $event === [] || !isset($event['request_status']) || $event['request_status'] === 'started') { return; } diff --git a/tests/security_functions_test.php b/tests/security_functions_test.php index f5bcee4..ea43d51 100644 --- a/tests/security_functions_test.php +++ b/tests/security_functions_test.php @@ -198,6 +198,12 @@ function audit_test_assert_same($expected, $actual, $message) { 'Credentials embedded in a URI must be redacted.' ); +$original_backtrack_limit = ini_get('pcre.backtrack_limit'); +ini_set('pcre.backtrack_limit', '0'); +$failed_redaction = audit_redact_cli_arguments(['https://user:must-not-leak@example.com/path']); +ini_set('pcre.backtrack_limit', (string) $original_backtrack_limit); +audit_test_assert_same('[REDACTED]', $failed_redaction[0], 'URI redaction failures must fail closed.'); + audit_test_assert_same("'=1+1", audit_csv_safe_cell('=1+1'), 'Spreadsheet formulas must be neutralized.'); $deep = []; @@ -335,6 +341,11 @@ function audit_test_assert_same($expected, $actual, $message) { audit_test_assert_same('', file_get_contents($temporary_log), 'Empty audit events must not create external records.'); audit_test_assert_same([], $audit_test_external_updates, 'Empty audit events must not update delivery status.'); +$audit_test_external_event = ['id' => 999]; +audit_deliver_external_event(999); +audit_test_assert_same('', file_get_contents($temporary_log), 'Events without a request status must not create external records.'); +audit_test_assert_same([], $audit_test_external_updates, 'Events without a request status must not update delivery status.'); + unlink($temporary_log); print "Security helper tests passed.\n"; From 7d8ec8f5b1a0e21881a9ea016c516984febcaea0 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 20:22:02 -0700 Subject: [PATCH 06/10] fix: fail closed across audit redaction paths --- CHANGELOG.md | 1 + audit.php | 4 ++-- audit_functions.php | 18 +++++++++++------- tests/security_functions_test.php | 11 ++++++++++- 4 files changed, 24 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d56164f..a398a4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ * feature: Finalize request outcomes and expose external-log delivery status * issue: Harden external file logging, retention, malformed records, and replication * issue#38: Graph Template table does not exist +* issue:#66: Restore the advertised PHP 7.4 runtime floor and fail-closed redaction * issue: If the audit log does not exist or is not set, set it and create it * issue: Audit assumes that all selected_items are numeric resulting in fatal error * feature: Support for Cacti 1.3 diff --git a/audit.php b/audit.php index e4d848a..b748ac4 100644 --- a/audit.php +++ b/audit.php @@ -271,14 +271,14 @@ function audit_render_value($value): string { } function audit_purge(): void { - $protected = db_fetch_cell("SELECT COUNT(*) + $protected = db_fetch_cell_prepared("SELECT COUNT(*) FROM audit_log WHERE EXISTS ( SELECT 1 FROM audit_syslog_delivery WHERE audit_syslog_delivery.audit_id = audit_log.id AND audit_syslog_delivery.state IN ('pending', 'retry', 'dead_letter') - )"); + )", []); db_execute("DELETE FROM audit_log WHERE NOT EXISTS ( diff --git a/audit_functions.php b/audit_functions.php index 898bfae..f433f65 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -138,11 +138,13 @@ function audit_process_page_data(string $page, $drop_action, array $selected_ite } /** - * @param mixed $key - * @return int|false + * @param mixed $key + * @return int */ function audit_is_sensitive_key($key) { - return preg_match('/(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', (string) $key); + $matched = preg_match('/(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', (string) $key); + + return $matched === false ? 1 : $matched; } /** @@ -178,13 +180,15 @@ function audit_redact_sensitive_value($value) { return $value; } - if (preg_match('/-----BEGIN (?:[A-Z ]+ )?PRIVATE KEY-----/', $value) || - preg_match('/^(?:Bearer|Basic)\s+[A-Za-z0-9+\/_=.-]+$/i', trim($value)) || - preg_match('/^[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{20,}$/', trim($value))) { + $private_key = preg_match('/-----BEGIN (?:[A-Z ]+ )?PRIVATE KEY-----/', $value); + $authorization = preg_match('/^(?:Bearer|Basic)\s+[A-Za-z0-9+\/_=.-]+$/i', trim($value)); + $token = preg_match('/^[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{20,}\.[A-Za-z0-9_-]{20,}$/', trim($value)); + + if ($private_key !== 0 || $authorization !== 0 || $token !== 0) { return '[REDACTED]'; } - return preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $value); + return preg_replace('#^([a-z][a-z0-9+.-]*://[^:/@\s]+):[^@\s]+@#i', '$1:[REDACTED]@', $value) ?? '[REDACTED]'; } /** diff --git a/tests/security_functions_test.php b/tests/security_functions_test.php index ea43d51..f865eee 100644 --- a/tests/security_functions_test.php +++ b/tests/security_functions_test.php @@ -200,9 +200,18 @@ function audit_test_assert_same($expected, $actual, $message) { $original_backtrack_limit = ini_get('pcre.backtrack_limit'); ini_set('pcre.backtrack_limit', '0'); -$failed_redaction = audit_redact_cli_arguments(['https://user:must-not-leak@example.com/path']); +$failed_redaction = audit_redact_cli_arguments(['https://user:must-not-leak@example.com/path']); +$failed_post_redaction = audit_redact_sensitive_data([ + 'password' => 'must-not-leak', + 'nested' => ['api_token' => 'must-not-leak'], +]); +$failed_value_redaction = audit_redact_sensitive_value('https://user:must-not-leak@example.com/path'); ini_set('pcre.backtrack_limit', (string) $original_backtrack_limit); audit_test_assert_same('[REDACTED]', $failed_redaction[0], 'URI redaction failures must fail closed.'); +audit_test_assert_same('[REDACTED]', $failed_post_redaction['password'], 'Sensitive key matching failures must fail closed.'); +audit_test_assert_same('[REDACTED]', $failed_post_redaction['nested']['api_token'], 'Nested sensitive key matching failures must fail closed.'); +audit_test_assert_same('[REDACTED]', $failed_value_redaction, 'Sensitive value matching failures must fail closed.'); +audit_test_assert_same('[REDACTED]', audit_syslog_cef_event_field($failed_value_redaction), 'Failed redaction must remain safe through CEF formatting.'); audit_test_assert_same("'=1+1", audit_csv_safe_cell('=1+1'), 'Spreadsheet formulas must be neutralized.'); From 9b828da369e397a8bb304755819372a8da868beb Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 20:31:53 -0700 Subject: [PATCH 07/10] fix: close audit redaction failure paths --- audit.php | 4 +-- audit_functions.php | 22 ++++++++++++++-- phpstan/stubs/cacti.stubs.php | 1 + .../PreparedStatementConsistencyTest.php | 13 ++++++++++ tests/security_functions_test.php | 25 ++++++++++++++++--- 5 files changed, 58 insertions(+), 7 deletions(-) diff --git a/audit.php b/audit.php index b748ac4..ff37799 100644 --- a/audit.php +++ b/audit.php @@ -280,13 +280,13 @@ function audit_purge(): void { AND audit_syslog_delivery.state IN ('pending', 'retry', 'dead_letter') )", []); - db_execute("DELETE FROM audit_log + db_execute_prepared("DELETE FROM audit_log WHERE NOT EXISTS ( SELECT 1 FROM audit_syslog_delivery WHERE audit_syslog_delivery.audit_id = audit_log.id AND audit_syslog_delivery.state IN ('pending', 'retry', 'dead_letter') - )"); + )", []); $purged = db_affected_rows(); audit_record_event('audit.log.purged', [ diff --git a/audit_functions.php b/audit_functions.php index f433f65..a423516 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -245,13 +245,31 @@ function audit_redact_cli_arguments(array $arguments): array { continue; } - if (preg_match('/^(--?[^=]*(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)[^=]*)=(.*)$/i', $argument, $matches)) { + $inline_match = preg_match('/^(--?[^=]*(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)[^=]*)=(.*)$/i', $argument, $matches); + + if ($inline_match === false) { + $redacted[] = '[REDACTED]'; + $redact_next = strpos($argument, '=') === false; + + continue; + } + + if ($inline_match === 1) { $redacted[] = $matches[1] . '=[REDACTED]'; continue; } - if (preg_match('/^--?[^=]*(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', $argument)) { + $key_match = preg_match('/^--?[^=]*(?:pass(?:word)?|phrase|token|secret|api[_-]?key|private[_-]?key|community|credential|authorization|authentication)/i', $argument); + + if ($key_match === false) { + $redacted[] = '[REDACTED]'; + $redact_next = true; + + continue; + } + + if ($key_match === 1) { $redacted[] = $argument; $redact_next = true; diff --git a/phpstan/stubs/cacti.stubs.php b/phpstan/stubs/cacti.stubs.php index cc7c8bd..60c5119 100644 --- a/phpstan/stubs/cacti.stubs.php +++ b/phpstan/stubs/cacti.stubs.php @@ -242,6 +242,7 @@ function sanitize_search_string(mixed $string): string { } function html_escape_request_var(string $name): string { + return ''; } // ----- Logging / Misc ------------------------------------------------ diff --git a/tests/Security/PreparedStatementConsistencyTest.php b/tests/Security/PreparedStatementConsistencyTest.php index 9a0c8cc..22bbcf8 100644 --- a/tests/Security/PreparedStatementConsistencyTest.php +++ b/tests/Security/PreparedStatementConsistencyTest.php @@ -13,6 +13,19 @@ */ describe('prepared statement consistency in audit', function () { + it('keeps the audit purge delete on the prepared helper', function () { + $contents = file_get_contents(__DIR__ . '/../../audit.php'); + + expect($contents)->not->toBeFalse(); + + $matched = preg_match('/function audit_purge\(\): void \{(?.*?)\n\}/s', (string) $contents, $matches); + $body = $matched === 1 && isset($matches['body']) ? $matches['body'] : ''; + + expect($matched)->toBe(1) + ->and($body)->toContain('db_execute_prepared("DELETE FROM audit_log') + ->and($body)->not->toMatch('/\bdb_execute\s*\(/'); + }); + it('documents database helper usage in all plugin files', function () { $targetFiles = [ 'audit.php', diff --git a/tests/security_functions_test.php b/tests/security_functions_test.php index f865eee..8d88cd7 100644 --- a/tests/security_functions_test.php +++ b/tests/security_functions_test.php @@ -45,13 +45,23 @@ function db_fetch_assoc_prepared($sql, $params = []) { }, $realm_ids); } -function db_fetch_row_prepared(string $sql, array $params = []): array|false { +/** + * @param string $sql + * @param array $params + * @return array|false + */ +function db_fetch_row_prepared($sql, $params = []) { global $audit_test_external_event; return $audit_test_external_event; } -function db_execute_prepared(string $sql, array $params = []): bool { +/** + * @param string $sql + * @param array $params + * @return bool + */ +function db_execute_prepared($sql, $params = []) { global $audit_test_external_updates; $audit_test_external_updates[] = ['sql' => $sql, 'params' => $params]; @@ -59,7 +69,11 @@ function db_execute_prepared(string $sql, array $params = []): bool { return true; } -function read_config_option(string $name): mixed { +/** + * @param string $name + * @return mixed + */ +function read_config_option($name) { global $audit_test_config_options; return $audit_test_config_options[$name] ?? ''; @@ -201,6 +215,8 @@ function audit_test_assert_same($expected, $actual, $message) { $original_backtrack_limit = ini_get('pcre.backtrack_limit'); ini_set('pcre.backtrack_limit', '0'); $failed_redaction = audit_redact_cli_arguments(['https://user:must-not-leak@example.com/path']); +$failed_inline_cli = audit_redact_cli_arguments(['--password=must-not-leak']); +$failed_separated_cli = audit_redact_cli_arguments(['--api-token', 'must-not-leak']); $failed_post_redaction = audit_redact_sensitive_data([ 'password' => 'must-not-leak', 'nested' => ['api_token' => 'must-not-leak'], @@ -208,6 +224,9 @@ function audit_test_assert_same($expected, $actual, $message) { $failed_value_redaction = audit_redact_sensitive_value('https://user:must-not-leak@example.com/path'); ini_set('pcre.backtrack_limit', (string) $original_backtrack_limit); audit_test_assert_same('[REDACTED]', $failed_redaction[0], 'URI redaction failures must fail closed.'); +audit_test_assert_same('[REDACTED]', $failed_inline_cli[0], 'Inline CLI key-matching failures must fail closed.'); +audit_test_assert_same('[REDACTED]', $failed_separated_cli[0], 'Separated CLI key-matching failures must redact the option.'); +audit_test_assert_same('[REDACTED]', $failed_separated_cli[1], 'Separated CLI key-matching failures must redact the value.'); audit_test_assert_same('[REDACTED]', $failed_post_redaction['password'], 'Sensitive key matching failures must fail closed.'); audit_test_assert_same('[REDACTED]', $failed_post_redaction['nested']['api_token'], 'Nested sensitive key matching failures must fail closed.'); audit_test_assert_same('[REDACTED]', $failed_value_redaction, 'Sensitive value matching failures must fail closed.'); From c8c682d4c98ecbf4b84050e4ec7af24f504c58c8 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 20:44:40 -0700 Subject: [PATCH 08/10] test: cover failed audit query results --- CHANGELOG.md | 2 +- audit_functions.php | 8 ++-- audit_syslog.php | 3 +- setup.php | 8 ++-- tests/Security/Php74CompatibilityTest.php | 17 ++++++-- tests/Security/SetupStructureTest.php | 28 ++++++++++++ tests/bootstrap.php | 20 +++++++-- tests/security_functions_test.php | 24 +++++++++++ tests/syslog_queue_test.php | 52 +++++++++++++++++++++++ 9 files changed, 147 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a398a4a..90090fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,7 @@ * feature: Finalize request outcomes and expose external-log delivery status * issue: Harden external file logging, retention, malformed records, and replication * issue#38: Graph Template table does not exist -* issue:#66: Restore the advertised PHP 7.4 runtime floor and fail-closed redaction +* issue#66: Restore the advertised PHP 7.4 runtime floor and fail-closed redaction * issue: If the audit log does not exist or is not set, set it and create it * issue: Audit assumes that all selected_items are numeric resulting in fatal error * feature: Support for Cacti 1.3 diff --git a/audit_functions.php b/audit_functions.php index a423516..cfca9c8 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -49,7 +49,8 @@ function audit_process_page_data(string $page, $drop_action, array $selected_ite WHERE id IN (?)', [$item]); - if (is_array($result)) { + if (cacti_sizeof($result)) { + /** @var array> $result */ foreach ($result as &$row) { $row['snmp'] = ($row['snmp'] == 1) ? 'UP' : 'Down'; $row['up'] = ($row['up'] == 1) ? 'Yes' : 'No'; @@ -501,7 +502,7 @@ function audit_deliver_external_event(int $id): void { $event = db_fetch_row_prepared('SELECT * FROM audit_log WHERE id = ?', [$id]); - if (!is_array($event) || $event === [] || !isset($event['request_status']) || $event['request_status'] === 'started') { + if (!cacti_sizeof($event) || !isset($event['request_status']) || $event['request_status'] === 'started') { return; } @@ -540,7 +541,8 @@ function audit_retry_external_logs(): void { ORDER BY id LIMIT 100"); - if (is_array($events)) { + if (cacti_sizeof($events)) { + /** @var array> $events */ foreach ($events as $event) { $message = audit_external_log_format(audit_external_event_data($event), $format); $delivery = audit_append_external_log($path, $message); diff --git a/audit_syslog.php b/audit_syslog.php index 5de9ffa..91cd59c 100644 --- a/audit_syslog.php +++ b/audit_syslog.php @@ -883,7 +883,8 @@ function audit_process_syslog_queue(): void { []); $socket = null; - if (is_array($deliveries)) { + if (cacti_sizeof($deliveries)) { + /** @var array> $deliveries */ foreach ($deliveries as $delivery) { $delivery_config = audit_syslog_delivery_config($config, $delivery); $result = audit_syslog_send_event($delivery, $delivery_config, $socket); diff --git a/setup.php b/setup.php index 4b3d3a8..b1550ea 100644 --- a/setup.php +++ b/setup.php @@ -62,7 +62,8 @@ function audit_setup_realms(bool $grant_installing_user = false): void { AND file IN (?, ?)', ['audit', 'audit.php', 'audit_manage.php']); - if (is_array($realm_ids)) { + if (cacti_sizeof($realm_ids)) { + /** @var array> $realm_ids */ foreach ($realm_ids as $realm) { db_execute_prepared('REPLACE INTO user_auth_realm (user_id, realm_id) @@ -81,7 +82,8 @@ function audit_remove_deprecated_realms(): void { AND file = ?', ['audit', 'audit_purge.php']); - if (is_array($realms)) { + if (cacti_sizeof($realms)) { + /** @var array> $realms */ foreach ($realms as $realm) { $realm_id = $realm['id'] + 100; @@ -424,7 +426,7 @@ function audit_upgrade_event_schema($rcnn_id = false): void { */ function plugin_audit_version(): array { global $config; - $info = parse_ini_file($config['base_path'] . '/plugins/audit/INFO', true); + $info = @parse_ini_file($config['base_path'] . '/plugins/audit/INFO', true); $plugin_info = is_array($info) ? ($info['info'] ?? null) : null; return is_array($plugin_info) ? $plugin_info : []; diff --git a/tests/Security/Php74CompatibilityTest.php b/tests/Security/Php74CompatibilityTest.php index a102174..89bbb96 100644 --- a/tests/Security/Php74CompatibilityTest.php +++ b/tests/Security/Php74CompatibilityTest.php @@ -16,10 +16,18 @@ 'setup.php', ]; $hasPhp8OnlyType = static function (string $contents): bool { - $withoutComments = preg_replace('#/\*.*?\*/|//[^\r\n]*#s', '', $contents); + $withoutComments = ''; - if ($withoutComments === null) { - return true; + foreach (token_get_all("toBeFalse() ->and($hasPhp8OnlyType('function nativeMixed(mixed $value) {}'))->toBeTrue() - ->and($hasPhp8OnlyType('function nativeUnion($value): int|false {}'))->toBeTrue(); + ->and($hasPhp8OnlyType('function nativeUnion($value): int|false {}'))->toBeTrue() + ->and($hasPhp8OnlyType("function url() { return 'https://example.test'; } function nativeMixed(mixed \$value) {}"))->toBeTrue(); }); it('keeps the compatibility floor explicit in plugin metadata', function () { diff --git a/tests/Security/SetupStructureTest.php b/tests/Security/SetupStructureTest.php index e3799d5..2fc9d44 100644 --- a/tests/Security/SetupStructureTest.php +++ b/tests/Security/SetupStructureTest.php @@ -9,8 +9,11 @@ // Verify setup.php defines required plugin hooks and info function. +require_once __DIR__ . '/../bootstrap.php'; + describe('audit setup.php structure', function () { $source = file_get_contents(realpath(__DIR__ . '/../../setup.php')); + require_once __DIR__ . '/../../setup.php'; it('defines plugin_audit_install function', function () use ($source) { expect($source)->toContain('function plugin_audit_install'); @@ -32,6 +35,31 @@ it('returns an array when plugin info is missing or malformed', function () use ($source) { expect($source)->toContain("\$info['info'] ?? null"); expect($source)->toContain('is_array($plugin_info) ? $plugin_info : []'); + + $GLOBALS['config']['base_path'] = '/definitely-missing-audit-test-path'; + set_error_handler(static function (): bool { + return true; + }); + + try { + expect(plugin_audit_version())->toBe([]); + } finally { + restore_error_handler(); + } + }); + + it('fails closed when realm queries fail', function () { + $GLOBALS['__test_db_calls'] = []; + $GLOBALS['__test_config_options']['admin_user'] = '1'; + $GLOBALS['__test_db_fetch_assoc_prepared_result'] = false; + + audit_setup_realms(false); + audit_remove_deprecated_realms(); + + expect($GLOBALS['__test_db_calls'])->toBe([]); + + $GLOBALS['__test_db_fetch_assoc_prepared_result'] = []; + $GLOBALS['__test_config_options'] = []; }); it('INFO file defines name and version keys', function () { diff --git a/tests/bootstrap.php b/tests/bootstrap.php index e268ffd..41c39a4 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -12,7 +12,9 @@ * can be loaded in isolation without the full Cacti application. */ -$GLOBALS['__test_db_calls'] = []; +$GLOBALS['__test_db_calls'] = []; +$GLOBALS['__test_db_fetch_assoc_prepared_result'] = []; +$GLOBALS['__test_config_options'] = []; if (!function_exists('db_execute')) { function db_execute($sql) { @@ -38,7 +40,7 @@ function db_fetch_assoc($sql) { if (!function_exists('db_fetch_assoc_prepared')) { function db_fetch_assoc_prepared($sql, $params = []) { - return []; + return $GLOBALS['__test_db_fetch_assoc_prepared_result']; } } @@ -102,9 +104,21 @@ function api_plugin_db_table_create($plugin, $table, $data) { } } +if (!function_exists('api_plugin_register_realm')) { + /** + * @param string $plugin + * @param string $file + * @param string $display + * @param int $grant + * @return void + */ + function api_plugin_register_realm($plugin, $file, $display, $grant = 0) { + } +} + if (!function_exists('read_config_option')) { function read_config_option($name, $force = false) { - return ''; + return $GLOBALS['__test_config_options'][$name] ?? ''; } } diff --git a/tests/security_functions_test.php b/tests/security_functions_test.php index 8d88cd7..81d10f4 100644 --- a/tests/security_functions_test.php +++ b/tests/security_functions_test.php @@ -9,6 +9,7 @@ $audit_test_realm_query_failure = false; $audit_test_object_query_failure = false; $audit_test_external_event = false; +$audit_test_external_events = false; $audit_test_external_updates = []; $audit_test_config_options = []; @@ -45,6 +46,24 @@ function db_fetch_assoc_prepared($sql, $params = []) { }, $realm_ids); } +/** + * @param string $sql + * @return array|false + */ +function db_fetch_assoc($sql) { + global $audit_test_external_events; + + return $audit_test_external_events; +} + +/** + * @param mixed $value + * @return int + */ +function cacti_sizeof($value) { + return is_array($value) ? count($value) : 0; +} + /** * @param string $sql * @param array $params @@ -374,6 +393,11 @@ function audit_test_assert_same($expected, $actual, $message) { audit_test_assert_same('', file_get_contents($temporary_log), 'Events without a request status must not create external records.'); audit_test_assert_same([], $audit_test_external_updates, 'Events without a request status must not update delivery status.'); +$audit_test_external_events = false; +audit_retry_external_logs(); +audit_test_assert_same('', file_get_contents($temporary_log), 'Failed retry queries must not append external records.'); +audit_test_assert_same([], $audit_test_external_updates, 'Failed retry queries must not update delivery status.'); + unlink($temporary_log); print "Security helper tests passed.\n"; diff --git a/tests/syslog_queue_test.php b/tests/syslog_queue_test.php index d535cf5..bcfa588 100644 --- a/tests/syslog_queue_test.php +++ b/tests/syslog_queue_test.php @@ -17,6 +17,7 @@ 'audit_syslog_batch_size' => '10', 'audit_syslog_pending_age_warning' => '900', 'audit_syslog_dead_letter_warning' => '1', + 'audit_syslog_health_state' => 'healthy', 'audit_syslog_tls_ca_file' => '', 'audit_syslog_tls_client_cert' => '', 'audit_syslog_tls_client_key' => '' @@ -28,6 +29,7 @@ 'event_uuid' => '32e0a97d-d9e8-4abc-8f41-2bbbc50793ca', 'request_status' => 'completed' ]; +$audit_queue_deliveries = false; function read_config_option($name) { global $audit_queue_settings; @@ -45,6 +47,33 @@ function db_fetch_row_prepared($sql, $params = []) { return $audit_queue_event; } +/** + * @param string $sql + * @param array $params + * @return array|false + */ +function db_fetch_assoc_prepared($sql, $params = []) { + global $audit_queue_deliveries; + + return $audit_queue_deliveries; +} + +/** + * @param string $sql + * @return array + */ +function db_fetch_row($sql) { + return []; +} + +/** + * @param string $sql + * @return string + */ +function db_fetch_cell($sql) { + return ''; +} + function db_execute_prepared($sql, $params = []) { global $audit_queue_calls; @@ -67,6 +96,24 @@ function db_affected_rows() { return $audit_queue_affected_rows; } +/** + * @param mixed $message + * @param mixed $also_print + * @param mixed $log_type + * @param mixed $level + * @return void + */ +function cacti_log($message, $also_print = false, $log_type = '', $level = 0) { +} + +/** + * @param mixed $name + * @param mixed $value + * @return void + */ +function set_config_option($name, $value) { +} + function cacti_sizeof($value) { return is_array($value) ? count($value) : 0; } @@ -127,6 +174,11 @@ function audit_queue_assert($condition, $message) { audit_queue_assert(strpos($audit_queue_calls[0]['params'][5], "\n") === false, 'Stored delivery errors must be bounded to one safe line.'); +$calls_before_failed_fetch = $audit_queue_calls; +audit_process_syslog_queue(); +audit_queue_assert($audit_queue_calls === $calls_before_failed_fetch, + 'A failed Syslog queue fetch must not attempt a delivery update.'); + $audit_queue_calls = []; $delivery['attempts'] = 4; audit_syslog_update_delivery($delivery, $failure, $config); From bd6acf4426ea0b14fa3e476032ca4e764f475dcf Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 20:54:37 -0700 Subject: [PATCH 09/10] fix: omit failed audit object lookups --- audit_functions.php | 55 ++++++++++++++++++------------- phpstan/index.php | 3 ++ phpstan/stubs/index.php | 3 ++ tests/index.php | 3 ++ tests/security_functions_test.php | 18 +++++++--- 5 files changed, 55 insertions(+), 27 deletions(-) create mode 100644 phpstan/index.php create mode 100644 phpstan/stubs/index.php create mode 100644 tests/index.php diff --git a/audit_functions.php b/audit_functions.php index cfca9c8..38d9a9e 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -6,6 +6,17 @@ function audit_user_is_admin(): bool { return api_plugin_user_realm_auth('audit_manage.php'); } +/** + * @param array>> $objects + * @param array>|false $result + */ +function audit_append_page_objects(array &$objects, $result): void { + if (cacti_sizeof($result)) { + /** @var array> $result */ + $objects[] = $result; + } +} + /** * @param array $selected_items * @param mixed $drop_action @@ -18,27 +29,27 @@ function audit_process_page_data(string $page, $drop_action, array $selected_ite case 'host.php': // loop over array and perform query for each item foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT id AS host_id,site_id,description,hostname,status,status_fail_date AS last_failed_date,status_rec_date AS last_recovered_date + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT id AS host_id,site_id,description,hostname,status,status_fail_date AS last_failed_date,status_rec_date AS last_recovered_date FROM host WHERE id IN (?)', - [$item]); + [$item])); } break; case 'host_templates.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM host_template WHERE id IN (?)', - [$item]); + [$item])); } break; case 'templates_export.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name FROM graph_templates + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM graph_templates WHERE id IN (?)', - [$item]); + [$item])); } break; @@ -63,72 +74,72 @@ function audit_process_page_data(string $page, $drop_action, array $selected_ite break; case 'graph_templates.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM graph_templates WHERE id IN (?)', - [$item]); + [$item])); } break; case 'thold.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT id,name_cache AS THOLD_NAME,data_source_name AS Data_Source + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT id,name_cache AS THOLD_NAME,data_source_name AS Data_Source FROM thold_data WHERE id IN (?)', - [$item]); + [$item])); } break; case 'data_sources.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('select name_cache AS Data_Source_Name,active from data_template_data + audit_append_page_objects($objects, db_fetch_assoc_prepared('select name_cache AS Data_Source_Name,active from data_template_data WHERE local_data_id IN (?)', - [$item]); + [$item])); } break; case 'data_templates.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM data_template WHERE id IN (?)', - [$item]); + [$item])); } break; case 'aggregate_templates.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM aggregate_graph_template WHERE id IN (?)', - [$item]); + [$item])); } break; case 'thold_templates.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM thold_template WHERE id IN (?)', - [$item]); + [$item])); } break; case 'user_admin.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT username + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT username FROM user_auth WHERE id IN (?)', - [$item]); + [$item])); } break; case 'user_group_admin.php': foreach ($selected_items as $item) { - $objects[] = db_fetch_assoc_prepared('SELECT name + audit_append_page_objects($objects, db_fetch_assoc_prepared('SELECT name FROM user_auth_group WHERE id IN (?)', - [$item]); + [$item])); } break; diff --git a/phpstan/index.php b/phpstan/index.php new file mode 100644 index 0000000..4e67c6b --- /dev/null +++ b/phpstan/index.php @@ -0,0 +1,3 @@ + Date: Mon, 17 Aug 2026 21:09:32 -0700 Subject: [PATCH 10/10] fix: fail closed on remaining PCRE errors --- .phpstan.neon | 4 ++-- audit_functions.php | 4 +++- audit_syslog.php | 11 ++++++++--- phpstan/stubs/{cacti.stubs.php => cacti.stub} | 0 tests/Security/Php74CompatibilityTest.php | 11 +++++++++++ tests/security_functions_test.php | 2 ++ tests/syslog_functions_test.php | 11 +++++++++++ 7 files changed, 37 insertions(+), 6 deletions(-) rename phpstan/stubs/{cacti.stubs.php => cacti.stub} (100%) diff --git a/.phpstan.neon b/.phpstan.neon index 6087a8c..ae6f006 100644 --- a/.phpstan.neon +++ b/.phpstan.neon @@ -22,7 +22,7 @@ parameters: - '*.po' - '*.pot' scanFiles: - - phpstan/stubs/cacti.stubs.php + - phpstan/stubs/cacti.stub level: 8 treatPhpDocTypesAsCertain: false reportUnmatchedIgnoredErrors: false @@ -44,4 +44,4 @@ parameters: - identifier: booleanAnd.rightAlwaysFalse - identifier: booleanAnd.rightAlwaysTrue - identifier: property.notFound - - identifier: parameterByRef.unusedType \ No newline at end of file + - identifier: parameterByRef.unusedType diff --git a/audit_functions.php b/audit_functions.php index 38d9a9e..e2d88cc 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -460,7 +460,9 @@ function audit_external_log_format(array $data, string $format = 'json'): string function audit_csv_safe_cell($value): string { $value = (string) $value; - if (preg_match('/^[=+\-@]/', ltrim($value))) { + $formula = preg_match('/^[=+\-@]/', ltrim($value)); + + if ($formula !== 0) { return "'" . $value; } diff --git a/audit_syslog.php b/audit_syslog.php index 91cd59c..f66e3fa 100644 --- a/audit_syslog.php +++ b/audit_syslog.php @@ -83,8 +83,13 @@ function audit_syslog_valid_receiver(string $receiver): bool { } function audit_syslog_valid_header_value(string $value, int $maximum): bool { - return $value !== '' && strlen($value) <= $maximum && - !preg_match('/[^\\x21-\\x7e]|[\\[\\]="]/', $value); + if ($value === '' || strlen($value) > $maximum) { + return false; + } + + $invalid = preg_match('/[^\\x21-\\x7e]|[\\[\\]="]/', $value); + + return $invalid === 0; } /** @@ -421,7 +426,7 @@ function audit_syslog_cef_payload(array $event, array $config): string { 'cs2Label' => 'Target', 'cs2' => trim(($event['target_type'] ?? '') . ':' . ($event['target_id'] ?? ''), ':'), 'cs3Label' => 'Node ID', - 'cs3' => $config['node_id'], + 'cs3' => audit_syslog_cef_event_field($config['node_id']), 'cn1Label' => 'Poller ID', 'cn1' => $config['poller_id'], 'cs4Label' => 'Submitted Data', diff --git a/phpstan/stubs/cacti.stubs.php b/phpstan/stubs/cacti.stub similarity index 100% rename from phpstan/stubs/cacti.stubs.php rename to phpstan/stubs/cacti.stub diff --git a/tests/Security/Php74CompatibilityTest.php b/tests/Security/Php74CompatibilityTest.php index 89bbb96..11ff159 100644 --- a/tests/Security/Php74CompatibilityTest.php +++ b/tests/Security/Php74CompatibilityTest.php @@ -71,4 +71,15 @@ expect($info['info']['compat'] ?? null)->toBe('1.2.20'); }); + + it('keeps developer-only PHP excluded from the runtime lint sweep', function () { + $workflow = file_get_contents(__DIR__ . '/../../.github/workflows/code-quality.yml'); + $config = file_get_contents(__DIR__ . '/../../.phpstan.neon'); + + expect($workflow)->not->toBeFalse() + ->and($workflow)->toContain("-path './phpstan' -prune") + ->and($workflow)->toContain("-path './tests' -prune") + ->and($config)->not->toBeFalse() + ->and($config)->toContain('phpstan/stubs/cacti.stub'); + }); }); diff --git a/tests/security_functions_test.php b/tests/security_functions_test.php index 576c275..dd2185c 100644 --- a/tests/security_functions_test.php +++ b/tests/security_functions_test.php @@ -249,6 +249,7 @@ function audit_test_assert_same($expected, $actual, $message) { 'nested' => ['api_token' => 'must-not-leak'], ]); $failed_value_redaction = audit_redact_sensitive_value('https://user:must-not-leak@example.com/path'); +$failed_csv_redaction = audit_csv_safe_cell('=must-not-execute'); ini_set('pcre.backtrack_limit', (string) $original_backtrack_limit); audit_test_assert_same('[REDACTED]', $failed_redaction[0], 'URI redaction failures must fail closed.'); audit_test_assert_same('[REDACTED]', $failed_inline_cli[0], 'Inline CLI key-matching failures must fail closed.'); @@ -258,6 +259,7 @@ function audit_test_assert_same($expected, $actual, $message) { audit_test_assert_same('[REDACTED]', $failed_post_redaction['nested']['api_token'], 'Nested sensitive key matching failures must fail closed.'); audit_test_assert_same('[REDACTED]', $failed_value_redaction, 'Sensitive value matching failures must fail closed.'); audit_test_assert_same('[REDACTED]', audit_syslog_cef_event_field($failed_value_redaction), 'Failed redaction must remain safe through CEF formatting.'); +audit_test_assert_same("'=must-not-execute", $failed_csv_redaction, 'CSV formula matching failures must fail closed.'); audit_test_assert_same("'=1+1", audit_csv_safe_cell('=1+1'), 'Spreadsheet formulas must be neutralized.'); diff --git a/tests/syslog_functions_test.php b/tests/syslog_functions_test.php index 5dae55e..86e5740 100644 --- a/tests/syslog_functions_test.php +++ b/tests/syslog_functions_test.php @@ -38,6 +38,17 @@ function audit_syslog_test_config($overrides = []) { return audit_syslog_config(array_merge($values, $overrides)); } +$original_backtrack_limit = ini_get('pcre.backtrack_limit'); +ini_set('pcre.backtrack_limit', '0'); +$failed_header_config = audit_syslog_test_config([ + 'format' => 'cef', + 'node_id' => "node\x01identifier" +]); +$failed_header_payload = audit_syslog_cef_payload(audit_syslog_test_event(), $failed_header_config); +ini_set('pcre.backtrack_limit', (string) $original_backtrack_limit); +audit_syslog_test_assert(!$failed_header_config['valid'], 'Header matching failures must invalidate the Syslog configuration.'); +audit_syslog_test_assert(strpos($failed_header_payload, "\x01") === false, 'CEF payloads must not retain control bytes from a failed header validation.'); + function audit_syslog_test_event() { return [ 'id' => 42,