Skip to content

hardening: migrate audit SQL helpers to prepared variants #49

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
somethingwithproof wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

hardening: migrate audit SQL helpers to prepared variants #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5cf077
security: migrate audit SQL helpers to prepared variants
somethingwithproof Mar 15, 2026
0a07152
fix: correct audit dependency table source
somethingwithproof Mar 16, 2026
9b48b03
test: add grok follow-up binding checks for audit
somethingwithproof Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 46 additions & 42 deletions audit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,28 +150,14 @@ function audit_purge() {
function audit_export_rows() {
process_request_vars();

/* form the 'where' clause for our main sql query */
if (get_request_var('filter') != '') {
$sql_where = 'WHERE (
page LIKE ' . db_qstr('%' . get_request_var('filter') . '%') . '
OR post LIKE ' . db_qstr('%' . get_request_var('filter') . '%') . ')';
} else {
$sql_where = '';
}
list($sql_where, $sql_params) = audit_build_filter_sql();

if (get_request_var('event_page') != '-1') {
$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');
}

$events = db_fetch_assoc("SELECT audit_log.*, user_auth.username
$events = db_fetch_assoc_prepared("SELECT audit_log.*, user_auth.username
FROM audit_log
LEFT JOIN user_auth
ON audit_log.user_id=user_auth.id
$sql_where");
$sql_where",
$sql_params);

if (cacti_sizeof($events)) {
header('Content-Disposition: attachment; filename=audit_export.csv');
Expand Down Expand Up @@ -252,6 +238,31 @@ function process_request_vars() {
/* ================= input validation ================= */
}

function audit_build_filter_sql() {
$sql_where = '';
$sql_params = array();

if (get_request_var('filter') != '') {
$sql_where = 'WHERE (
page LIKE ?
OR post LIKE ?)';
$sql_params[] = '%' . get_request_var('filter') . '%';
$sql_params[] = '%' . get_request_var('filter') . '%';
}

if (get_request_var('event_page') != '-1') {
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' page = ?';
$sql_params[] = 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 = ?';
$sql_params[] = get_request_var('user_id');
}

return array($sql_where, $sql_params);
}

function audit_log() {
global $item_rows;

Expand Down Expand Up @@ -284,7 +295,10 @@ function audit_log() {
<select id='event_page'>
<option value='-1'<?php print (get_request_var('event_page') == '-1' ? ' selected>':'>') . __('All', 'audit');?></option>
<?php
$pages = array_rekey(db_fetch_assoc('SELECT DISTINCT page FROM audit_log ORDER BY page'), 'page', 'page');
$pages = array_rekey(db_fetch_assoc_prepared('SELECT DISTINCT page
FROM audit_log
ORDER BY page',
array()), 'page', 'page');
if (cacti_sizeof($pages)) {
foreach ($pages as $page) {
print "<option value='" . $page . "'"; if (get_request_var('event_page') == $page) { print ' selected'; } print '>' . htmlspecialchars($page) . "</option>\n";
Expand All @@ -301,7 +315,10 @@ function audit_log() {
<option value='-1'<?php print (get_request_var('user_id') == '-1' ? ' selected>':'>') . __('All', 'audit');?></option>
<option value='0'<?php print (get_request_var('user_id') == '0' ? ' selected>':'>') . __('cli', 'audit');?></option>
<?php
$users = array_rekey(db_fetch_assoc('SELECT DISTINCT user_id FROM audit_log ORDER BY user_id'), 'user_id', 'user_id');
$users = array_rekey(db_fetch_assoc_prepared('SELECT DISTINCT user_id
FROM audit_log
ORDER BY user_id',
array()), 'user_id', 'user_id');
if (cacti_sizeof($users)) {
foreach ($users as $user) {
if ($user == 0) continue;
Expand Down Expand Up @@ -344,40 +361,28 @@ function audit_log() {

html_end_box();

/* form the 'where' clause for our main sql query */
if (get_request_var('filter') != '') {
$sql_where = 'WHERE (
page LIKE ' . db_qstr('%' . get_request_var('filter') . '%') . '
OR post LIKE ' . db_qstr('%' . get_request_var('filter') . '%') . ')';
} else {
$sql_where = '';
}

if (get_request_var('event_page') != '-1') {
$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') . ' page = ' . db_qstr(get_request_var('event_page'));
}
list($sql_where, $sql_params) = audit_build_filter_sql();

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');
}

$total_rows = db_fetch_cell("SELECT
$total_rows = db_fetch_cell_prepared("SELECT
COUNT(*)
FROM audit_log
LEFT JOIN user_auth
ON audit_log.user_id=user_auth.id
$sql_where");
$sql_where",
$sql_params);

$sql_order = get_order_string();
$sql_limit = ' LIMIT ' . ($rows*(get_request_var('page')-1)) . ',' . $rows;
$offset = ((int)$rows * ((int)get_request_var('page') - 1));
$sql_limit = ' LIMIT ' . $offset . ',' . (int)$rows;

$events = db_fetch_assoc("SELECT audit_log.*, user_auth.username
$events = db_fetch_assoc_prepared("SELECT audit_log.*, user_auth.username
FROM audit_log
LEFT JOIN user_auth
ON audit_log.user_id=user_auth.id
$sql_where
$sql_order
$sql_limit");
$sql_limit",
$sql_params);

$nav = html_nav_bar('audit.php?filter=' . get_request_var('filter'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 5, __('Audit Events', 'audit'), 'page', 'main');

Expand Down Expand Up @@ -463,4 +468,3 @@ function audit_log() {
<script type='text/javascript' src='plugins/audit/js/functions.js'></script>
<?php
}

34 changes: 21 additions & 13 deletions setup.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ function audit_check_upgrade() {

$info = plugin_audit_version();
$current = $info['version'];
$old = db_fetch_cell("SELECT version FROM plugin_config WHERE directory='audit'");
$old = db_fetch_cell_prepared('SELECT version
FROM plugin_config
WHERE directory = ?',
array('audit'));
if ($current != $old) {
if (api_plugin_is_enabled('audit')) {
# may sound ridiculous, but enables new hooks
Expand All @@ -83,16 +86,18 @@ function audit_check_upgrade() {
db_execute('ALTER TABLE audit_log ADD COLUMN IF NOT EXISTS object_data LONGBLOB');
}

db_execute("UPDATE plugin_config
SET version='$current'
WHERE directory='audit'");
db_execute_prepared('UPDATE plugin_config
SET version = ?
WHERE directory = ?',
array($current, 'audit'));

db_execute("UPDATE plugin_config SET
version='" . $info['version'] . "',
name='" . $info['longname'] . "',
author='" . $info['author'] . "',
webpage='" . $info['homepage'] . "'
WHERE directory='" . $info['name'] . "' ");
db_execute_prepared('UPDATE plugin_config
SET version = ?,
name = ?,
author = ?,
webpage = ?
WHERE directory = ?',
array($info['version'], $info['longname'], $info['author'], $info['homepage'], $info['name']));

/* hook for table replication */
api_plugin_register_hook('audit', 'replicate_out', 'audit_replicate_out', 'setup.php', '1');
Expand All @@ -106,8 +111,8 @@ function audit_check_dependencies($data) {
$class = $data['class'];

if ($class == 'all') {
if (!db_table_exists('alert_log', false, $rcnn_id)) {
$create = db_fetch_cell('SHOW CREATE TABLE autid_log');
if (!db_table_exists('audit_log', false, $rcnn_id)) {
$create = db_fetch_cell_prepared('SHOW CREATE TABLE audit_log', array());

db_execute($create, false, $rcnn_id);
Comment on lines 113 to 117
Copy link
Author

@somethingwithproof somethingwithproof Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. Fixed in 0a07152: corrected the dependency check to audit_log and aligned SHOW CREATE TABLE audit_log to the actual plugin schema. Also added regression coverage in tests/test_prepared_statements.php.

}
Expand Down Expand Up @@ -154,7 +159,10 @@ 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)) . ')');
$delete_before = time() - ($retention * 86400);
db_execute_prepared('DELETE FROM audit_log
WHERE event_time < FROM_UNIXTIME(?)',
array($delete_before));
$rows = db_affected_rows();
cacti_log('NOTE: Purged ' . $rows . ' Audit Log Records from Cacti', false, 'POLLER');
}
Expand Down
79 changes: 79 additions & 0 deletions tests/test_prepared_statements.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| Regression checks for prepared DB helper migration in audit plugin |
| |
| Run: php tests/test_prepared_statements.php |
+-------------------------------------------------------------------------+
*/

$pass = 0;
$fail = 0;

function assert_true($label, $value) {
global $pass, $fail;

if ($value) {
echo "PASS $label\n";
$pass++;
} else {
echo "FAIL $label\n";
$fail++;
}
}

$setup_contents = file_get_contents(__DIR__ . '/../setup.php');
$audit_contents = file_get_contents(__DIR__ . '/../audit.php');

assert_true('setup.php is readable', $setup_contents !== false);
assert_true('audit.php is readable', $audit_contents !== false);

$setup_contents = ($setup_contents === false ? '' : $setup_contents);
$audit_contents = ($audit_contents === false ? '' : $audit_contents);

assert_true(
'setup.php uses prepared plugin version lookup',
preg_match('/db_fetch_cell_prepared\s*\(\s*\'SELECT version\s+FROM plugin_config\s+WHERE directory = \?/s', $setup_contents) === 1
);
assert_true(
'setup.php binds audit directory parameter in version lookup',
preg_match('/db_fetch_cell_prepared\s*\(.*directory\s*=\s*\?.*,\s*(?:array\(\s*[\'"]audit[\'"]\s*\)|\[\s*[\'"]audit[\'"]\s*\])\s*\)/s', $setup_contents) === 1
);
assert_true(
'setup.php uses prepared plugin config updates',
preg_match_all('/db_execute_prepared\s*\(\s*\'UPDATE plugin_config/s', $setup_contents) >= 2
);
assert_true(
'setup.php uses prepared retention purge delete',
preg_match('/db_execute_prepared\s*\(\s*\'DELETE FROM audit_log\s+WHERE event_time < FROM_UNIXTIME\(\?\)/s', $setup_contents) === 1
);
assert_true(
'setup.php binds retention purge timestamp parameter',
preg_match('/db_execute_prepared\s*\(.*DELETE FROM audit_log[\s\S]*FROM_UNIXTIME\(\?\)[\s\S]*,\s*(?:array\(\s*\$delete_before\s*\)|\[\s*\$delete_before\s*\])\s*\)/s', $setup_contents) === 1
);
assert_true(
'setup.php dependency check references audit_log consistently',
preg_match('/db_table_exists\s*\(\s*[\'"]audit_log[\'"]/', $setup_contents) === 1
&& preg_match('/SHOW CREATE TABLE\s+audit_log/i', $setup_contents) === 1
&& preg_match('/\b(?:alert_log|autid_log)\b/i', $setup_contents) === 0
);
assert_true(
'audit.php uses prepared page/user selector queries',
preg_match_all('/db_fetch_assoc_prepared\s*\(\s*\'SELECT DISTINCT (?:page|user_id)/s', $audit_contents) >= 2
);
assert_true(
'audit.php uses prepared count and event fetch queries',
preg_match('/db_fetch_cell_prepared\s*\(\s*"SELECT\s+COUNT\(\*\)/s', $audit_contents) === 1 &&
preg_match_all('/db_fetch_assoc_prepared\s*\(\s*"SELECT audit_log\.\*, user_auth\.username/s', $audit_contents) >= 2
);
assert_true(
'audit.php has no raw db_fetch_cell/db_fetch_assoc calls',
preg_match('/\bdb_fetch_(?:cell|assoc)\s*\(/', $audit_contents) === 0
);

echo "\n";
echo "Results: $pass passed, $fail failed\n";

exit($fail > 0 ? 1 : 0);