From 29888db43baf28f8a14b1e9550743db8bc9ecf0c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Wed, 8 Apr 2026 22:59:40 -0700 Subject: [PATCH 1/3] refactor(php74): use null coalescing (??) and ??= operators Signed-off-by: Thomas Vincent --- audit.php | 22 +++++++++++----------- audit_functions.php | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/audit.php b/audit.php index dffed41..2aa7cd3 100644 --- a/audit.php +++ b/audit.php @@ -51,13 +51,13 @@ if ($data['action'] == 'cli') { $width = 'wide'; $output .= '
'; - $output .= '' . __('Page:', 'audit') . ' ' . $data['page'] . ''; - $output .= '
' . __('User:', 'audit') . ' ' . $data['user_agent'] . ''; - $output .= '
' . __('IP Address:', 'audit') . ' ' . $data['ip_address'] . ''; - $output .= '
' . __('Date:', 'audit') . ' ' . $data['event_time'] . ''; - $output .= '
' . __('Action:', 'audit') . ' ' . $data['action'] . ''; + $output .= '' . __('Page:', 'audit') . ' ' . html_escape($data['page']) . ''; + $output .= '
' . __('User:', 'audit') . ' ' . html_escape($data['user_agent']) . ''; + $output .= '
' . __('IP Address:', 'audit') . ' ' . html_escape($data['ip_address']) . ''; + $output .= '
' . __('Date:', 'audit') . ' ' . html_escape($data['event_time']) . ''; + $output .= '
' . __('Action:', 'audit') . ' ' . html_escape($data['action']) . ''; $output .= '
'; - $output .= '' . __('Script:', 'audit') . ' ' . $data['post'] . ''; + $output .= '' . __('Script:', 'audit') . ' ' . html_escape($data['post']) . ''; } elseif (cacti_sizeof($data)) { $attribs = json_decode($data['post']); @@ -74,11 +74,11 @@ } $output .= '
'; - $output .= '' . __('Page:', 'audit') . ' ' . $data['page'] . ''; - $output .= '
' . __('User:', 'audit') . ' ' . get_username($data['user_id']) . ''; - $output .= '
' . __('IP Address:', 'audit') . ' ' . $data['ip_address'] . ''; - $output .= '
' . __('Date:', 'audit') . ' ' . $data['event_time'] . ''; - $output .= '
' . __('Action:', 'audit') . ' ' . $data['action'] . ''; + $output .= '' . __('Page:', 'audit') . ' ' . html_escape($data['page']) . ''; + $output .= '
' . __('User:', 'audit') . ' ' . html_escape(get_username($data['user_id'])) . ''; + $output .= '
' . __('IP Address:', 'audit') . ' ' . html_escape($data['ip_address']) . ''; + $output .= '
' . __('Date:', 'audit') . ' ' . html_escape($data['event_time']) . ''; + $output .= '
' . __('Action:', 'audit') . ' ' . html_escape($data['action']) . ''; $output .= '
'; $output .= ''; diff --git a/audit_functions.php b/audit_functions.php index 6a93705..00a46d3 100644 --- a/audit_functions.php +++ b/audit_functions.php @@ -157,7 +157,7 @@ function audit_config_insert() { $post = json_encode($post); $page = basename($_SERVER['SCRIPT_NAME']); - $user_id = (isset($_SESSION['sess_user_id']) ? $_SESSION['sess_user_id'] : 0); + $user_id = ($_SESSION['sess_user_id'] ?? 0); $event_time = date('Y-m-d H:i:s'); /* Retrieve IP address */ From ba6a55b6929de0d108eee5ceef04dbb6962e4b72 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Wed, 8 Apr 2026 21:46:56 -0700 Subject: [PATCH 2/3] fix(sql): migrate interpolated queries to prepared statements Signed-off-by: Thomas Vincent --- setup.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.php b/setup.php index ca1af13..ec91a99 100644 --- a/setup.php +++ b/setup.php @@ -154,7 +154,7 @@ function audit_poller_bottom() { $retention = read_config_option('audit_retention'); if ($retention > 0) { - db_execute('DELETE FROM audit_log WHERE event_time < FROM_UNIXTIME(' . (time() - ($retention * 86400)) . ')'); + db_execute_prepared('DELETE FROM audit_log WHERE event_time < FROM_UNIXTIME(?)', array(time() - ($retention * 86400))); $rows = db_affected_rows(); cacti_log('NOTE: Purged ' . $rows . ' Audit Log Records from Cacti', false, 'POLLER'); } From 59bbe87fb9c25378c1060a3417a413d228f7b752 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 11 Apr 2026 13:41:48 -0700 Subject: [PATCH 3/3] fix(security): harden audit detail rendering --- audit.php | 18 +++---- audit_helpers.php | 34 +++++++++++++ .../test_detail_render_escaping.php | 45 +++++++++++++++++ tests/Unit/test_security_helpers.php | 47 ++++++++++++++++++ tests/e2e/test_security_wiring.php | 48 +++++++++++++++++++ 5 files changed, 184 insertions(+), 8 deletions(-) create mode 100644 audit_helpers.php create mode 100644 tests/Integration/test_detail_render_escaping.php create mode 100644 tests/Unit/test_security_helpers.php create mode 100644 tests/e2e/test_security_wiring.php diff --git a/audit.php b/audit.php index 2aa7cd3..02d4c47 100644 --- a/audit.php +++ b/audit.php @@ -24,6 +24,7 @@ chdir('../../'); include_once('./include/auth.php'); +require_once(__DIR__ . '/audit_helpers.php'); set_default_action(); @@ -98,9 +99,9 @@ } if (is_array($content)) { - $output .= '' . implode(',', $content) . ''; + $output .= ''; } else { - $output .= ''; + $output .= ''; } $i++; @@ -119,7 +120,7 @@ $output .= ''; foreach ($recordData as $record) { - $output .= ''; + $output .= ''; } } else { $output .= '
' . $field . '' . audit_html_escape($field) . '' . audit_html_escape(implode(',', $content)) . '' . $field . '' . $content . '' . audit_html_escape($field) . '' . audit_html_escape($content) . '
' . __('Record Data:', 'audit') . '
' . json_encode($record, JSON_PRETTY_PRINT) . '
' . audit_html_escape(json_encode($record, JSON_PRETTY_PRINT)) . '
'; @@ -163,8 +164,9 @@ function audit_export_rows() { $sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' page = ' . db_qstr(get_request_var('event_page')); } - if (!isempty_request_var('user_id') && get_request_var('user_id') > '-1') { - $sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . get_request_var('user_id'); + $user_id = audit_normalize_int(get_request_var('user_id'), -1); + if ($user_id > -1) { + $sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . $user_id; } $events = db_fetch_assoc("SELECT audit_log.*, user_auth.username @@ -357,8 +359,9 @@ function audit_log() { $sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' page = ' . db_qstr(get_request_var('event_page')); } - if (!isempty_request_var('user_id') && get_request_var('user_id') > '-1') { - $sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . get_request_var('user_id'); + $user_id = audit_normalize_int(get_request_var('user_id'), -1); + if ($user_id > -1) { + $sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . $user_id; } $total_rows = db_fetch_cell("SELECT @@ -463,4 +466,3 @@ function audit_log() { '); +$rendered_json = audit_html_escape(json_encode(array('payload' => ''))); + +assert_true( + 'field names are escaped before rendering', + strpos($rendered_field, ' 0 ? 1 : 0); diff --git a/tests/Unit/test_security_helpers.php b/tests/Unit/test_security_helpers.php new file mode 100644 index 0000000..6fb5f3c --- /dev/null +++ b/tests/Unit/test_security_helpers.php @@ -0,0 +1,47 @@ +alert(1)') === '<script>alert(1)</script>' +); + +echo "\n"; +echo "Results: $pass passed, $fail failed\n"; + +exit($fail > 0 ? 1 : 0); diff --git a/tests/e2e/test_security_wiring.php b/tests/e2e/test_security_wiring.php new file mode 100644 index 0000000..ddb88cd --- /dev/null +++ b/tests/e2e/test_security_wiring.php @@ -0,0 +1,48 @@ + 0 ? 1 : 0);