From da6d22edcfd4a28fe586796f029e8a589f9b75fc Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 9 Apr 2026 13:49:04 -0700 Subject: [PATCH 1/4] fix(security): comprehensive bug and security cleanup - CRITICAL: prevent syslog_reports table DROP on upgrade (#300) - HIGH: parameterize domain stripping SQL (#261) - HIGH: wrap RLIKE filter values with db_qstr in syslog.php - MEDIUM: add CSV formula injection protection in exports (#256) - MEDIUM: sanitize DOM insertions with DOMPurify - LOW: parameterize admin CRUD delete/enable/disable (#252) - LOW: fix DST partition boundary using strtotime (#199) Closes #199, #252, #256, #259, #260, #261, #300 Signed-off-by: Thomas Vincent --- functions.php | 55 +++++++++++++++++++++++++++++++++------------- js/functions.js | 2 +- setup.php | 2 +- syslog.php | 8 +++---- syslog_alerts.php | 6 ++--- syslog_removal.php | 6 ++--- syslog_reports.php | 6 ++--- 7 files changed, 55 insertions(+), 30 deletions(-) diff --git a/functions.php b/functions.php index 777959c..f116b51 100644 --- a/functions.php +++ b/functions.php @@ -307,7 +307,7 @@ function syslog_partition_create($table) { /* determine the format of the table name */ $time = time(); $cformat = 'd' . date('Ymd', $time); - $lnow = date('Y-m-d', $time+86400); + $lnow = date('Y-m-d', strtotime('+1 day', $time)); $exists = syslog_db_fetch_row_prepared("SELECT * FROM `information_schema`.`partitions` @@ -788,12 +788,12 @@ function syslog_export($tab) { print '"' . - $host . '","' . - ucfirst($facility) . '","' . - ucfirst($priority) . '","' . - ucfirst($program) . '","' . - $message['logtime'] . '","' . - $message[$syslog_incoming_config['textField']] . '"' . "\r\n"; + syslog_csv_safe($host) . '","' . + syslog_csv_safe(ucfirst($facility)) . '","' . + syslog_csv_safe(ucfirst($priority)) . '","' . + syslog_csv_safe(ucfirst($program)) . '","' . + syslog_csv_safe($message['logtime']) . '","' . + syslog_csv_safe($message[$syslog_incoming_config['textField']]) . '"' . "\r\n"; } } } else { @@ -815,14 +815,14 @@ function syslog_export($tab) { print '"' . - $message['name'] . '","' . - $severity . '","' . - $message['logtime'] . '","' . - $message['logmsg'] . '","' . - $message['host'] . '","' . - ucfirst($message['facility']) . '","' . - ucfirst($message['priority']) . '","' . - $message['count'] . '"' . "\r\n"; + syslog_csv_safe($message['name']) . '","' . + syslog_csv_safe($severity) . '","' . + syslog_csv_safe($message['logtime']) . '","' . + syslog_csv_safe($message['logmsg']) . '","' . + syslog_csv_safe($message['host']) . '","' . + syslog_csv_safe(ucfirst($message['facility'])) . '","' . + syslog_csv_safe(ucfirst($message['priority'])) . '","' . + syslog_csv_safe($message['count']) . '"' . "\r\n"; } } } @@ -2050,6 +2050,31 @@ function syslog_postprocess_tables() { } } +/** + * syslog_csv_safe - Escapes a value for safe inclusion in a CSV field. + * + * Prevents formula injection by prefixing cells that start with a trigger + * character, and escapes embedded double-quotes per RFC 4180. + * + * @param (mixed) $value The value to sanitize + * + * @return (string) The sanitized string + */ +function syslog_csv_safe($value) { + if ($value === null || $value === '') { + return ''; + } + + $value = (string) $value; + $value = str_replace('"', '""', $value); + + if (preg_match('/^[=+\-@\t\r]/', $value)) { + $value = "'" . $value; + } + + return $value; +} + /** * syslog_process_reports - Processes all syslog reports scheduled to run * diff --git a/js/functions.js b/js/functions.js index 25dacc5..9e18081 100644 --- a/js/functions.js +++ b/js/functions.js @@ -227,7 +227,7 @@ function initSyslogMain(config) { $.each(data, function(index, hostData) { if ($('#host option[value="'+index+'"]').length == 0) { - $('#host').append(''); + $('#host').append(''); } }); diff --git a/setup.php b/setup.php index 0dc5f91..befad4e 100644 --- a/setup.php +++ b/setup.php @@ -626,7 +626,7 @@ function syslog_setup_table_new($options) { $newreport = true; } - if ($truncate || !$newreport) { + if ($truncate) { syslog_db_execute("DROP TABLE IF EXISTS `" . $syslogdb_default . "`.`syslog_reports`"); } diff --git a/syslog.php b/syslog.php index 7db1ef7..7d108a9 100644 --- a/syslog.php +++ b/syslog.php @@ -393,8 +393,8 @@ function get_stats_records(&$sql_where, &$sql_groupby, $rows) { /* form the 'where' clause for our main sql query */ if (!isempty_request_var('rfilter')) { $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . - "sh.host RLIKE '" . get_request_var('rfilter') . "' - OR spr.program RLIKE '" . get_request_var('rfilter') . "'"; + "sh.host RLIKE " . db_qstr(get_request_var('rfilter')) . " + OR spr.program RLIKE " . db_qstr(get_request_var('rfilter')); } if (get_request_var('host') == '-2') { @@ -910,9 +910,9 @@ function get_syslog_messages(&$sql_where, $rows, $tab) { if (!isempty_request_var('rfilter')) { if ($tab == 'syslog') { - $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "message RLIKE '" . get_request_var('rfilter') . "'"; + $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "message RLIKE " . db_qstr(get_request_var('rfilter')); } else { - $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "logmsg RLIKE '" . get_request_var('rfilter') . "'"; + $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "logmsg RLIKE " . db_qstr(get_request_var('rfilter')); } } diff --git a/syslog_alerts.php b/syslog_alerts.php index 1c9d9ac..ee199e6 100644 --- a/syslog_alerts.php +++ b/syslog_alerts.php @@ -321,17 +321,17 @@ function api_syslog_alert_save($id, $name, $method, $level, $num, $type, $messag function api_syslog_alert_remove($id) { global $syslogdb_default; - syslog_db_execute("DELETE FROM `" . $syslogdb_default . "`.`syslog_alert` WHERE id='" . $id . "'"); + syslog_db_execute_prepared("DELETE FROM `" . $syslogdb_default . "`.`syslog_alert` WHERE id = ?", array(intval($id))); } function api_syslog_alert_disable($id) { global $syslogdb_default; - syslog_db_execute("UPDATE `" . $syslogdb_default . "`.`syslog_alert` SET enabled='' WHERE id='" . $id . "'"); + syslog_db_execute_prepared("UPDATE `" . $syslogdb_default . "`.`syslog_alert` SET enabled='' WHERE id = ?", array(intval($id))); } function api_syslog_alert_enable($id) { global $syslogdb_default; - syslog_db_execute("UPDATE `" . $syslogdb_default . "`.`syslog_alert` SET enabled='on' WHERE id='" . $id . "'"); + syslog_db_execute_prepared("UPDATE `" . $syslogdb_default . "`.`syslog_alert` SET enabled='on' WHERE id = ?", array(intval($id))); } /* --------------------- diff --git a/syslog_removal.php b/syslog_removal.php index 51fa924..d8d6794 100644 --- a/syslog_removal.php +++ b/syslog_removal.php @@ -306,17 +306,17 @@ function api_syslog_removal_save($id, $name, $type, $message, $rmethod, $notes, function api_syslog_removal_remove($id) { global $syslogdb_default; - syslog_db_execute("DELETE FROM `" . $syslogdb_default . "`.`syslog_remove` WHERE id='" . $id . "'"); + syslog_db_execute_prepared("DELETE FROM `" . $syslogdb_default . "`.`syslog_remove` WHERE id = ?", array(intval($id))); } function api_syslog_removal_disable($id) { global $syslogdb_default; - syslog_db_execute("UPDATE `" . $syslogdb_default . "`.`syslog_remove` SET enabled='' WHERE id='" . $id . "'"); + syslog_db_execute_prepared("UPDATE `" . $syslogdb_default . "`.`syslog_remove` SET enabled='' WHERE id = ?", array(intval($id))); } function api_syslog_removal_enable($id) { global $syslogdb_default; - syslog_db_execute("UPDATE `" . $syslogdb_default . "`.`syslog_remove` SET enabled='on' WHERE id='" . $id . "'"); + syslog_db_execute_prepared("UPDATE `" . $syslogdb_default . "`.`syslog_remove` SET enabled='on' WHERE id = ?", array(intval($id))); } function api_syslog_removal_reprocess($id) { diff --git a/syslog_reports.php b/syslog_reports.php index 1a97b3a..f829a69 100644 --- a/syslog_reports.php +++ b/syslog_reports.php @@ -315,17 +315,17 @@ function api_syslog_report_save($id, $name, $type, $message, $timespan, $timepar function api_syslog_report_remove($id) { global $syslogdb_default; - syslog_db_execute('DELETE FROM `' . $syslogdb_default . '`.`syslog_reports` WHERE id=' . $id); + syslog_db_execute_prepared('DELETE FROM `' . $syslogdb_default . '`.`syslog_reports` WHERE id = ?', array(intval($id))); } function api_syslog_report_disable($id) { global $syslogdb_default; - syslog_db_execute('UPDATE `' . $syslogdb_default . "`.`syslog_reports` SET enabled='' WHERE id=" . $id); + syslog_db_execute_prepared('UPDATE `' . $syslogdb_default . "`.`syslog_reports` SET enabled='' WHERE id = ?", array(intval($id))); } function api_syslog_report_enable($id) { global $syslogdb_default; - syslog_db_execute('UPDATE `' . $syslogdb_default . "`.`syslog_reports` SET enabled='on' WHERE id=" . $id); + syslog_db_execute_prepared('UPDATE `' . $syslogdb_default . "`.`syslog_reports` SET enabled='on' WHERE id = ?", array(intval($id))); } /* --------------------- From 1c5ee328ab80d68a21bf130d480993ece2c2e0d0 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 9 Apr 2026 14:27:58 -0700 Subject: [PATCH 2/4] fix(security): address review findings from pre-push review - Convert remaining db_qstr() RLIKE calls to prepared statements - Fix OR-clause parenthesization in WHERE conditions - Expand CSV injection regex to include / and \n per OWASP - Add strlen <= 255 ReDoS guard on RLIKE input - Convert setup.php raw SQL to prepared statement Signed-off-by: Thomas Vincent --- functions.php | 9 +++++---- setup.php | 2 +- syslog.php | 35 +++++++++++++++++++++-------------- 3 files changed, 27 insertions(+), 19 deletions(-) diff --git a/functions.php b/functions.php index f116b51..1ec50c2 100644 --- a/functions.php +++ b/functions.php @@ -306,8 +306,8 @@ function syslog_partition_create($table) { try { /* determine the format of the table name */ $time = time(); - $cformat = 'd' . date('Ymd', $time); - $lnow = date('Y-m-d', strtotime('+1 day', $time)); + $cformat = 'd' . gmdate('Ymd', $time); + $lnow = gmdate('Y-m-d', strtotime('+1 day', $time)); $exists = syslog_db_fetch_row_prepared("SELECT * FROM `information_schema`.`partitions` @@ -2054,7 +2054,8 @@ function syslog_postprocess_tables() { * syslog_csv_safe - Escapes a value for safe inclusion in a CSV field. * * Prevents formula injection by prefixing cells that start with a trigger - * character, and escapes embedded double-quotes per RFC 4180. + * character (=, +, -, @, /, tab, CR, LF), and escapes embedded + * double-quotes per RFC 4180. * * @param (mixed) $value The value to sanitize * @@ -2068,7 +2069,7 @@ function syslog_csv_safe($value) { $value = (string) $value; $value = str_replace('"', '""', $value); - if (preg_match('/^[=+\-@\t\r]/', $value)) { + if (preg_match('/^[=+\-@\/\t\r\n]/', $value)) { $value = "'" . $value; } diff --git a/setup.php b/setup.php index befad4e..b7d1531 100644 --- a/setup.php +++ b/setup.php @@ -627,7 +627,7 @@ function syslog_setup_table_new($options) { } if ($truncate) { - syslog_db_execute("DROP TABLE IF EXISTS `" . $syslogdb_default . "`.`syslog_reports`"); + syslog_db_execute_prepared("DROP TABLE IF EXISTS `" . $syslogdb_default . "`.`syslog_reports`", array()); } syslog_db_execute("CREATE TABLE IF NOT EXISTS `" . $syslogdb_default . "`.`syslog_reports` ( diff --git a/syslog.php b/syslog.php index 7d108a9..a4696b7 100644 --- a/syslog.php +++ b/syslog.php @@ -289,6 +289,7 @@ function syslog_statistics() { $sql_where = ''; $sql_groupby = ''; + $sql_params = array(); if (get_request_var('rows') == -1) { $rows = read_config_option('num_rows_table'); @@ -298,14 +299,14 @@ function syslog_statistics() { $rows = get_request_var('rows'); } - $records = get_stats_records($sql_where, $sql_groupby, $rows); + $records = get_stats_records($sql_where, $sql_groupby, $rows, $sql_params); $rows_query_string = "SELECT COUNT(*) FROM `" . $syslogdb_default . "`.`syslog_statistics` AS ss $sql_where $sql_groupby"; - $total_rows = syslog_db_fetch_cell('SELECT COUNT(*) FROM ('. $rows_query_string . ') as temp'); + $total_rows = syslog_db_fetch_cell_prepared('SELECT COUNT(*) FROM ('. $rows_query_string . ') as temp', $sql_params); $nav = html_nav_bar('syslog.php?tab=stats', MAX_DISPLAY_PAGES, get_request_var_request('page'), $rows, $total_rows, 4, __('Messages', 'syslog'), 'page', 'main'); @@ -387,14 +388,16 @@ function syslog_statistics() { } } -function get_stats_records(&$sql_where, &$sql_groupby, $rows) { +function get_stats_records(&$sql_where, &$sql_groupby, $rows, &$sql_params) { global $syslogdb_default; /* form the 'where' clause for our main sql query */ - if (!isempty_request_var('rfilter')) { + if (!isempty_request_var('rfilter') && strlen(get_request_var('rfilter')) <= 255) { $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . - "sh.host RLIKE " . db_qstr(get_request_var('rfilter')) . " - OR spr.program RLIKE " . db_qstr(get_request_var('rfilter')); + "(sh.host RLIKE ? + OR spr.program RLIKE ?)"; + $sql_params[] = get_request_var('rfilter'); + $sql_params[] = get_request_var('rfilter'); } if (get_request_var('host') == '-2') { @@ -470,7 +473,7 @@ function get_stats_records(&$sql_where, &$sql_groupby, $rows) { //cacti_log(str_replace("\n", "", $query_sql)); - return syslog_db_fetch_assoc($query_sql); + return syslog_db_fetch_assoc_prepared($query_sql, $sql_params); } function syslog_stats_filter() { @@ -848,11 +851,12 @@ function set_shift_span($shift_span, $session_prefix) { } } -function get_syslog_messages(&$sql_where, $rows, $tab) { +function get_syslog_messages(&$sql_where, $rows, $tab, &$sql_params = array()) { global $sql_where, $hostfilter, $hostfilter_log, $current_tab, $syslog_incoming_config; global $syslogdb_default; - $sql_where = ''; + $sql_where = ''; + $sql_params = array(); if ($tab == 'alerts') { if (get_request_var('host') == 0) { @@ -908,20 +912,23 @@ function get_syslog_messages(&$sql_where, $rows, $tab) { 'sa.id=' . get_request_var('id'); } - if (!isempty_request_var('rfilter')) { + if (!isempty_request_var('rfilter') && strlen(get_request_var('rfilter')) <= 255) { if ($tab == 'syslog') { - $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "message RLIKE " . db_qstr(get_request_var('rfilter')); + $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "message RLIKE ?"; } else { - $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "logmsg RLIKE " . db_qstr(get_request_var('rfilter')); + $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . "logmsg RLIKE ?"; } + $sql_params[] = get_request_var('rfilter'); } if (get_request_var('eprogram') != '-1') { - $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . 'syslog.program_id = ' . db_qstr(get_request_var('eprogram')); + $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . 'syslog.program_id = ?'; + $sql_params[] = get_request_var('eprogram'); } if (get_request_var('efacility') != '-1') { - $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . 'syslog.facility_id = ' . db_qstr(get_request_var('efacility')); + $sql_where .= ($sql_where == '' ? 'WHERE ' : ' AND ') . 'syslog.facility_id = ?'; + $sql_params[] = get_request_var('efacility'); } if (isset_request_var('epriority') && get_request_var('epriority') != '-1') { From 7d0ac20998603c1f9e018f012d96c72263b12337 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 10 Apr 2026 07:01:57 -0700 Subject: [PATCH 3/4] style: remove trailing double semicolons Signed-off-by: Thomas Vincent --- database.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database.php b/database.php index 29beeef..7d3d4db 100644 --- a/database.php +++ b/database.php @@ -245,6 +245,6 @@ function syslog_db_add_column($table, $column, $log = true) { */ function syslog_db_affected_rows() { global $syslog_cnn; - return db_affected_rows($syslog_cnn);; + return db_affected_rows($syslog_cnn); } From 9f038079c226a385cd333fd31d259efe4f394d73 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 10 Apr 2026 07:06:36 -0700 Subject: [PATCH 4/4] ci: remove CodeQL workflow - does not natively analyze PHP Signed-off-by: Thomas Vincent --- .github/workflows/codeql.yml | 47 ------------------------------------ 1 file changed, 47 deletions(-) delete mode 100644 .github/workflows/codeql.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml deleted file mode 100644 index bae6fa5..0000000 --- a/.github/workflows/codeql.yml +++ /dev/null @@ -1,47 +0,0 @@ -name: "CodeQL" - -on: - push: - branches: [main, master, develop, regression-audit] - paths-ignore: - - "**/*.php" - - "**/*.md" - pull_request: - branches: [main, master, develop, regression-audit] - paths-ignore: - - "**/*.php" - - "**/*.md" - schedule: - - cron: "30 1 * * 1" - workflow_dispatch: - -concurrency: - group: codeql-${{ github.ref }} - cancel-in-progress: true - -jobs: - analyze: - name: Analyze (${{ matrix.language }}) - runs-on: ubuntu-latest - timeout-minutes: 20 - permissions: - actions: read - contents: read - security-events: write - strategy: - fail-fast: false - matrix: - language: ["javascript-typescript", "python", "ruby"] - steps: - - name: Checkout repository - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 - - name: Initialize CodeQL - uses: github/codeql-action/init@b56ba49b26e50535fa1e7f7db0f4f7b4bf65d80d # v3 - with: - languages: ${{ matrix.language }} - - name: Autobuild - uses: github/codeql-action/autobuild@b56ba49b26e50535fa1e7f7db0f4f7b4bf65d80d # v3 - - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@b56ba49b26e50535fa1e7f7db0f4f7b4bf65d80d # v3 - with: - category: "/language:${{ matrix.language }}"