Skip to content

Commit 59bbe87

Browse files
somethingwithproofsomethingwithproof
committed
fix(security): harden audit detail rendering
1 parent ba6a55b commit 59bbe87

5 files changed

Lines changed: 184 additions & 8 deletions

File tree

‎audit.php‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
chdir('../../');
2626
include_once('./include/auth.php');
27+
require_once(__DIR__ . '/audit_helpers.php');
2728

2829
set_default_action();
2930

@@ -98,9 +99,9 @@
9899
}
99100

100101
if (is_array($content)) {
101-
$output .= '<td style="font-weight:bold;white-space:nowrap;">' . $field . '</td><td">' . implode(',', $content) . '</td>';
102+
$output .= '<td style="font-weight:bold;white-space:nowrap;">' . audit_html_escape($field) . '</td><td>' . audit_html_escape(implode(',', $content)) . '</td>';
102103
} else {
103-
$output .= '<td style="font-weight:bold;white-space:nowrap;">' . $field . '</td><td>' . $content . '</td>';
104+
$output .= '<td style="font-weight:bold;white-space:nowrap;">' . audit_html_escape($field) . '</td><td>' . audit_html_escape($content) . '</td>';
104105
}
105106

106107
$i++;
@@ -119,7 +120,7 @@
119120
$output .= '<tr><td colspan="' . ($columns * 2) . '"><b>' . __('Record Data:', 'audit') . '</b></td></tr>';
120121

121122
foreach ($recordData as $record) {
122-
$output .= '<tr><td colspan="' . ($columns * 2) . '"><pre>' . json_encode($record, JSON_PRETTY_PRINT) . '</pre></td></tr>';
123+
$output .= '<tr><td colspan="' . ($columns * 2) . '"><pre>' . audit_html_escape(json_encode($record, JSON_PRETTY_PRINT)) . '</pre></td></tr>';
123124
}
124125
} else {
125126
$output .= '</table>';
@@ -163,8 +164,9 @@ function audit_export_rows() {
163164
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' page = ' . db_qstr(get_request_var('event_page'));
164165
}
165166

166-
if (!isempty_request_var('user_id') && get_request_var('user_id') > '-1') {
167-
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . get_request_var('user_id');
167+
$user_id = audit_normalize_int(get_request_var('user_id'), -1);
168+
if ($user_id > -1) {
169+
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . $user_id;
168170
}
169171

170172
$events = db_fetch_assoc("SELECT audit_log.*, user_auth.username
@@ -357,8 +359,9 @@ function audit_log() {
357359
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' page = ' . db_qstr(get_request_var('event_page'));
358360
}
359361

360-
if (!isempty_request_var('user_id') && get_request_var('user_id') > '-1') {
361-
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . get_request_var('user_id');
362+
$user_id = audit_normalize_int(get_request_var('user_id'), -1);
363+
if ($user_id > -1) {
364+
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' user_id = ' . $user_id;
362365
}
363366

364367
$total_rows = db_fetch_cell("SELECT
@@ -463,4 +466,3 @@ function audit_log() {
463466
<script type='text/javascript' src='plugins/audit/js/functions.js'></script>
464467
<?php
465468
}
466-

‎audit_helpers.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| Shared helpers for audit request normalization and output escaping. |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
if (!function_exists('audit_normalize_int')) {
11+
function audit_normalize_int($value, $default = -1) {
12+
if (is_int($value)) {
13+
return $value;
14+
}
15+
16+
if (is_string($value) && preg_match('/^-?[0-9]+$/', $value)) {
17+
return (int)$value;
18+
}
19+
20+
return (int)$default;
21+
}
22+
}
23+
24+
if (!function_exists('audit_html_escape')) {
25+
function audit_html_escape($value) {
26+
$value = (string)$value;
27+
28+
if (function_exists('html_escape')) {
29+
return html_escape($value);
30+
}
31+
32+
return htmlspecialchars($value, ENT_QUOTES, 'UTF-8');
33+
}
34+
}

‎tests/Integration/test_detail_render_escaping.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| Integration checks for audit detail escaping helpers. |
7+
| |
8+
| Run: php tests/integration/test_detail_render_escaping.php |
9+
+-------------------------------------------------------------------------+
10+
*/
11+
12+
$pass = 0;
13+
$fail = 0;
14+
15+
function assert_true($label, $value) {
16+
global $pass, $fail;
17+
18+
if ($value) {
19+
echo "PASS $label\n";
20+
$pass++;
21+
} else {
22+
echo "FAIL $label\n";
23+
$fail++;
24+
}
25+
}
26+
27+
require_once __DIR__ . '/../../audit_helpers.php';
28+
29+
$rendered_field = audit_html_escape('"><img src=x onerror=alert(1)>');
30+
$rendered_json = audit_html_escape(json_encode(array('payload' => '<svg onload=alert(1)>')));
31+
32+
assert_true(
33+
'field names are escaped before rendering',
34+
strpos($rendered_field, '<img') === false && strpos($rendered_field, '&lt;img') !== false
35+
);
36+
37+
assert_true(
38+
'record data json is escaped before rendering',
39+
strpos($rendered_json, '<svg') === false && strpos($rendered_json, '&lt;svg') !== false
40+
);
41+
42+
echo "\n";
43+
echo "Results: $pass passed, $fail failed\n";
44+
45+
exit($fail > 0 ? 1 : 0);

‎tests/Unit/test_security_helpers.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| Unit checks for audit security helpers. |
7+
| |
8+
| Run: php tests/unit/test_security_helpers.php |
9+
+-------------------------------------------------------------------------+
10+
*/
11+
12+
$pass = 0;
13+
$fail = 0;
14+
15+
function assert_true($label, $value) {
16+
global $pass, $fail;
17+
18+
if ($value) {
19+
echo "PASS $label\n";
20+
$pass++;
21+
} else {
22+
echo "FAIL $label\n";
23+
$fail++;
24+
}
25+
}
26+
27+
require_once __DIR__ . '/../../audit_helpers.php';
28+
29+
assert_true(
30+
'integer normalization keeps valid integers',
31+
audit_normalize_int('42', -1) === 42
32+
);
33+
34+
assert_true(
35+
'integer normalization rejects mixed payloads',
36+
audit_normalize_int('0 OR 1=1', -1) === -1
37+
);
38+
39+
assert_true(
40+
'html escaping encodes script payloads',
41+
audit_html_escape('<script>alert(1)</script>') === '&lt;script&gt;alert(1)&lt;/script&gt;'
42+
);
43+
44+
echo "\n";
45+
echo "Results: $pass passed, $fail failed\n";
46+
47+
exit($fail > 0 ? 1 : 0);

‎tests/e2e/test_security_wiring.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
| |
6+
| End-to-end wiring checks for audit security fixes. |
7+
| |
8+
| Run: php tests/e2e/test_security_wiring.php |
9+
+-------------------------------------------------------------------------+
10+
*/
11+
12+
$pass = 0;
13+
$fail = 0;
14+
15+
function assert_true($label, $value) {
16+
global $pass, $fail;
17+
18+
if ($value) {
19+
echo "PASS $label\n";
20+
$pass++;
21+
} else {
22+
echo "FAIL $label\n";
23+
$fail++;
24+
}
25+
}
26+
27+
$source = file_get_contents(__DIR__ . '/../../audit.php');
28+
29+
assert_true(
30+
'user_id filter uses integer normalization before SQL concatenation',
31+
substr_count($source, "audit_normalize_int(get_request_var('user_id'), -1)") === 2
32+
);
33+
34+
assert_true(
35+
'detail popup escapes field names and values',
36+
strpos($source, "audit_html_escape(\$field)") !== false &&
37+
strpos($source, "audit_html_escape(\$content)") !== false
38+
);
39+
40+
assert_true(
41+
'record data json is escaped before rendering',
42+
strpos($source, "audit_html_escape(json_encode(\$record, JSON_PRETTY_PRINT))") !== false
43+
);
44+
45+
echo "\n";
46+
echo "Results: $pass passed, $fail failed\n";
47+
48+
exit($fail > 0 ? 1 : 0);

0 commit comments

Comments
 (0)