Skip to content

Commit 4c5d0a8

Browse files
somethingwithproofTheWitness
somethingwithproof
and
TheWitness
authored
hardening: prepared statements, PHP 7.4 idioms, and security fixes (#324)
* refactor(php74): use null coalescing (??) and ??= operators Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * security: fix SQLi, XSS, credential exposure, and insecure deserialization Replace bare unserialize() with cacti_unserialize() plus array validation in mactrack_view_macs.php. The original call allowed PHP object injection on a guest-accessible page. Strip 9 SNMP credential columns (community strings, SNMPv3 usernames, passwords, auth/priv protocols, passphrases) from the guest-accessible CSV export in mactrack_view_devices.php. Any user with Viewer realm could download all network device SNMP credentials. Addresses GHSA-9829-w9mx-2cgm. Replace all 27 unsafe LIKE string interpolation sites across 9 files with db_qstr() quoting. Also fix mactrack_create_sql_filter() in lib/mactrack_functions.php. Six of the affected files are guest-accessible, making filter SQLi pre-authentication. Replace raw db_qstr()-less CSV value interpolation in the import processors of mactrack_devices.php and mactrack_device_types.php. CSV cell values were concatenated directly into INSERT VALUES and WHERE clauses with only single-quote stripping (bypassable via backslash escaping). Replace get_request_var('filter') with html_escape_request_var() in 11 HTML input value attributes to prevent reflected XSS. Wrap 4 SNMP response variables with html_escape() in mactrack_devices.php to prevent stored XSS via rogue SNMP agents. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * refactor: extract plugin_get_rows_per_page helper to DRY filter boilerplate Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * chore: add copilot instructions and CI workflow Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> * fix(security): escape filter option labels * qa: Fixing php-cs-fixer issues --------- Signed-off-by: Thomas Vincent <thomasvincent@gmail.com> Co-authored-by: TheWitness <thewitness@cacti.net>
1 parent cf2b82e commit 4c5d0a8

44 files changed

Lines changed: 505 additions & 300 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎.github/copilot-instructions.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
# Cacti mactrack Plugin AI Instructions
2+
3+
## Project Overview
4+
This is a Cacti plugin. It integrates with the Cacti monitoring platform via the plugin hook architecture.
5+
6+
## Technology Stack
7+
- PHP 7.4+ (targeting Cacti 1.2.x compatibility)
8+
- MySQL/MariaDB via Cacti's DB abstraction layer
9+
- PSR-12 coding standards
10+
11+
## Key Rules
12+
- Use prepared statements (db_execute_prepared, db_fetch_row_prepared, etc.) for ALL queries with variables
13+
- Use get_request_var() / get_filter_request_var() for ALL user input, never raw $_REQUEST/$_GET/$_POST
14+
- Use html_escape() / htmlspecialchars() for ALL output of DB/user values in HTML context
15+
- Use cacti_escapeshellarg() for ALL shell command arguments
16+
- No PHP 8.0+ features (str_contains, match, union types, named args) - target PHP 7.4
17+
- Use ?? and ??= operators (PHP 7.4) instead of isset() ternary patterns
18+
- All unserialize() calls must use allowed_classes => false
19+
20+
## Testing
21+
- Tests in tests/ directory
22+
- Use Pest PHP or PHPUnit
23+
- php -l lint check required before commit

‎.github/workflows/plugin-ci-workflow.yml‎

Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
# +-------------------------------------------------------------------------+
2+
# | Copyright (C) 2004-2026 The Cacti Group |
3+
# | |
4+
# | This program is free software; you can redistribute it and/or |
5+
# | modify it under the terms of the GNU General Public License |
6+
# | as published by the Free Software Foundation; either version 2 |
7+
# | of the License, or (at your option) any later version. |
8+
# | |
9+
# | This program is distributed in the hope that it will be useful, |
10+
# | but WITHOUT ANY WARRANTY; without even the implied warranty of |
11+
# | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
12+
# | GNU General Public License for more details. |
13+
# +-------------------------------------------------------------------------+
14+
# | Cacti: The Complete RRDtool-based Graphing Solution |
15+
# +-------------------------------------------------------------------------+
16+
# | This code is designed, written, and maintained by the Cacti Group. See |
17+
# | about.php and/or the AUTHORS file for specific developer information. |
18+
# +-------------------------------------------------------------------------+
19+
# | http://www.cacti.net/ |
20+
# +-------------------------------------------------------------------------+
21+
22+
name: Plugin Integration Tests
23+
24+
on:
25+
push:
26+
branches:
27+
- main
28+
- develop
29+
pull_request:
30+
branches:
31+
- main
32+
- develop
33+
34+
jobs:
35+
integration-test:
36+
runs-on: ${{ matrix.os }}
37+
38+
strategy:
39+
fail-fast: false
40+
matrix:
41+
php: ['8.1', '8.2', '8.3', '8.4']
42+
os: [ubuntu-latest]
43+
44+
services:
45+
mariadb:
46+
image: mariadb:10.6
47+
env:
48+
MYSQL_ROOT_PASSWORD: cactiroot
49+
MYSQL_DATABASE: cacti
50+
MYSQL_USER: cactiuser
51+
MYSQL_PASSWORD: cactiuser
52+
ports:
53+
- 3306:3306
54+
options: >-
55+
--health-cmd="mysqladmin ping"
56+
--health-interval=10s
57+
--health-timeout=5s
58+
--health-retries=3
59+
60+
name: PHP ${{ matrix.php }} Integration Test on ${{ matrix.os }}
61+
62+
steps:
63+
- name: Checkout Cacti
64+
uses: actions/checkout@v4
65+
with:
66+
repository: Cacti/cacti
67+
path: cacti
68+
69+
- name: Checkout mactrack Plugin
70+
uses: actions/checkout@v4
71+
with:
72+
path: cacti/plugins/mactrack
73+
74+
- name: Install PHP ${{ matrix.php }}
75+
uses: shivammathur/setup-php@v2
76+
with:
77+
php-version: ${{ matrix.php }}
78+
extensions: intl, mysql, gd, ldap, gmp, xml, curl, json, mbstring
79+
ini-values: "post_max_size=256M, max_execution_time=60, date.timezone=America/New_York"
80+
81+
- name: Check PHP version
82+
run: php -v
83+
84+
- name: Run apt-get update
85+
run: sudo apt-get update
86+
87+
- name: Install System Dependencies
88+
run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }}
89+
90+
- name: Start SNMPD Agent and Test
91+
run: |
92+
sudo systemctl start snmpd
93+
sudo snmpwalk -c public -v2c -On localhost .1.3.6.1.2.1.1
94+
95+
- name: Setup Permissions
96+
run: |
97+
sudo chown -R www-data:runner ${{ github.workspace }}/cacti
98+
sudo find ${{ github.workspace }}/cacti -type d -exec chmod 775 {} \;
99+
sudo find ${{ github.workspace }}/cacti -type f -exec chmod 664 {} \;
100+
sudo chmod +x ${{ github.workspace }}/cacti/cmd.php
101+
sudo chmod +x ${{ github.workspace }}/cacti/poller.php
102+
103+
- name: Create MySQL Config
104+
run: |
105+
echo -e "[client]\nuser = root\npassword = cactiroot\nhost = 127.0.0.1\n" > ~/.my.cnf
106+
cat ~/.my.cnf
107+
108+
- name: Initialize Cacti Database
109+
env:
110+
MYSQL_AUTH_USR: '--defaults-file=~/.my.cnf'
111+
run: |
112+
mysql $MYSQL_AUTH_USR -e 'CREATE DATABASE IF NOT EXISTS cacti;'
113+
mysql $MYSQL_AUTH_USR -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';"
114+
mysql $MYSQL_AUTH_USR -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';"
115+
mysql $MYSQL_AUTH_USR -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';"
116+
mysql $MYSQL_AUTH_USR -e "FLUSH PRIVILEGES;"
117+
mysql $MYSQL_AUTH_USR cacti < ${{ github.workspace }}/cacti/cacti.sql
118+
mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti
119+
120+
- name: Validate composer files
121+
run: |
122+
cd ${{ github.workspace }}/cacti
123+
if [ -f composer.json ]; then
124+
composer validate --strict || true
125+
fi
126+
127+
- name: Install Composer Dependencies
128+
run: |
129+
cd ${{ github.workspace }}/cacti
130+
if [ -f composer.json ]; then
131+
sudo composer install --prefer-dist --no-progress
132+
fi
133+
134+
- name: Create Cacti config.php
135+
run: |
136+
cat ${{ github.workspace }}/cacti/include/config.php.dist | \
137+
sed -r "s/localhost/127.0.0.1/g" | \
138+
sed -r "s/'cacti'/'cacti'/g" | \
139+
sed -r "s/'cactiuser'/'cactiuser'/g" | \
140+
sed -r "s/'cactiuser'/'cactiuser'/g" > ${{ github.workspace }}/cacti/include/config.php
141+
sudo chmod 664 ${{ github.workspace }}/cacti/include/config.php
142+
143+
- name: Configure Apache
144+
run: |
145+
cat << 'EOF' | sed 's#GITHUB_WORKSPACE#${{ github.workspace }}#g' > /tmp/cacti.conf
146+
<VirtualHost *:80>
147+
ServerAdmin webmaster@localhost
148+
DocumentRoot GITHUB_WORKSPACE/cacti
149+
150+
<Directory GITHUB_WORKSPACE/cacti>
151+
Options Indexes FollowSymLinks
152+
AllowOverride All
153+
Require all granted
154+
</Directory>
155+
156+
ErrorLog ${APACHE_LOG_DIR}/error.log
157+
CustomLog ${APACHE_LOG_DIR}/access.log combined
158+
</VirtualHost>
159+
EOF
160+
sudo cp /tmp/cacti.conf /etc/apache2/sites-available/000-default.conf
161+
sudo systemctl restart apache2
162+
163+
- name: Install Cacti via CLI
164+
run: |
165+
cd ${{ github.workspace }}/cacti
166+
sudo php cli/install_cacti.php --accept-eula --install --force
167+
168+
- name: Install mactrack Plugin
169+
run: |
170+
cd ${{ github.workspace }}/cacti
171+
sudo php cli/plugin_manage.php --plugin=mactrack --install --enable
172+
173+
# - name: import mactrack Plugin Sample Data
174+
# run: |
175+
# cd ${{ github.workspace }}/cacti/plugins/mactrack
176+
# sudo php cli_import.php --filename=.github/workflows/mactrack_sample_data.xml
177+
# if [ $? -ne 0 ]; then
178+
# echo "Failed to import Thold sample data"
179+
# exit 1
180+
# fi
181+
182+
- name: Check PHP Syntax for Plugin
183+
run: |
184+
cd ${{ github.workspace }}/cacti/plugins/mactrack
185+
if find . -name '*.php' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then
186+
echo "Syntax errors found!"
187+
exit 1
188+
fi
189+
190+
- name: Remove the plugins directory exclusion from the .phpstan.neon
191+
run: sed '/plugins/d' -i .phpstan.neon
192+
working-directory: ${{ github.workspace }}/cacti
193+
194+
- name: Mark composer scripts executable
195+
run: sudo chmod +x ${{ github.workspace }}/cacti/include/vendor/bin/*
196+
197+
- name: Run Linter on base code
198+
run: composer run-script lint ${{ github.workspace }}/cacti/plugins/mactrack
199+
working-directory: ${{ github.workspace }}/cacti
200+
201+
- name: Checking coding standards on base code
202+
run: composer run-script phpcsfixer ${{ github.workspace }}/cacti/plugins/mactrack
203+
working-directory: ${{ github.workspace }}/cacti
204+
205+
# - name: Run PHPStan at Level 6 on base code outside of Composer due to technical issues
206+
# run: ./include/vendor/bin/phpstan analyze --level 6 ${{ github.workspace }}/cacti/plugins/mactrack
207+
# working-directory: ${{ github.workspace }}/cacti
208+
209+
- name: Run Cacti Poller
210+
run: |
211+
cd ${{ github.workspace }}/cacti
212+
sudo php poller.php --poller=1 --force --debug
213+
if ! grep -q "SYSTEM STATS" log/cacti.log; then
214+
echo "Cacti poller did not finish successfully"
215+
cat log/cacti.log
216+
exit 1
217+
fi
218+
219+
- name: View Cacti Logs
220+
if: always()
221+
run: |
222+
if [ -f ${{ github.workspace }}/cacti/log/cacti.log ]; then
223+
echo "=== Cacti Log ==="
224+
sudo cat ${{ github.workspace }}/cacti/log/cacti.log
225+
fi

‎lib/mactrack_3com.php‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@
2323
*/
2424

2525
// register this functions scanning functions
26-
if (!isset($mactrack_scanning_functions)) {
27-
$mactrack_scanning_functions = [];
28-
}
26+
$mactrack_scanning_functions ??= [];
2927
array_push($mactrack_scanning_functions, 'get_3Com_dot1dTpFdbEntry_ports');
3028

3129
/* complete_3com_ifName
@@ -210,7 +208,7 @@ function get_3Com_base_dot1dTpFdbEntry_ports($site, &$device, &$ifInterfaces, $s
210208
if (isset($bridgePortIfIndexes[$port_key['port_number']])) {
211209
$brPortIfIndex = mactrack_arr_key($bridgePortIfIndexes, $port_key['port_number']);
212210
} else {
213-
$brPortIfIndex = isset($port_key['port_number']) ? $port_key['port_number'] : '';
211+
$brPortIfIndex = $port_key['port_number'] ?? '';
214212
}
215213
$brPortIfType = isset($ifInterfaces[$brPortIfIndex]['ifType']) ? $ifInterfaces[$brPortIfIndex]['ifType'] : '';
216214
} else {

‎lib/mactrack_aruba_oscx.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,10 @@
2323
*/
2424

2525
// register this functions scanning functions
26-
if (!isset($mactrack_scanning_functions)) {
27-
$mactrack_scanning_functions = [];
28-
}
26+
$mactrack_scanning_functions ??= [];
2927
array_push($mactrack_scanning_functions, 'get_aruba_oscx_switch_ports');
3028

31-
if (!isset($mactrack_scanning_functions_ip)) {
32-
$mactrack_scanning_functions_ip = [];
33-
}
29+
$mactrack_scanning_functions_ip ??= [];
3430
array_push($mactrack_scanning_functions_ip, 'get_aruba_oscx_arp_table');
3531

3632
function oscx_mac($mac) {
@@ -375,7 +371,7 @@ function get_aruba_oscx_dot1dTpFdbEntry_ports($site, &$device, &$ifInterfaces, $
375371

376372
// now set the real data
377373
$new_port_key_array[$i]['key'] = mactrack_arr_key($port_key, 'key');
378-
$new_port_key_array[$i]['port_number'] = isset($brPortIfIndex) ? $brPortIfIndex : '';
374+
$new_port_key_array[$i]['port_number'] = $brPortIfIndex ?? '';
379375
$new_port_key_array[$i]['port_name'] = mactrack_arr_key($ifInterfaces, $port_key['port_number']);
380376
$new_port_key_array[$i]['mac_address'] = oscx_mac($port_key['key']);
381377
$new_port_key_array[$i]['vlan_id'] = mactrack_arr_key($port_vlan_data, $brPortIfIndex);

‎lib/mactrack_cabletron.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@
2323
*/
2424

2525
// register this functions scanning functions
26-
if (!isset($mactrack_scanning_functions)) {
27-
$mactrack_scanning_functions = [];
28-
}
26+
$mactrack_scanning_functions ??= [];
2927
array_push($mactrack_scanning_functions, 'get_cabletron_switch_ports');
3028
array_push($mactrack_scanning_functions, 'get_repeater_rev4_ports');
3129

‎lib/mactrack_cisco.php‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,15 @@
2323
*/
2424

2525
// register this functions scanning functions
26-
if (!isset($mactrack_scanning_functions)) {
27-
$mactrack_scanning_functions = [];
28-
}
26+
$mactrack_scanning_functions ??= [];
2927
array_push($mactrack_scanning_functions, 'get_catalyst_dot1dTpFdbEntry_ports');
3028
array_push($mactrack_scanning_functions, 'get_IOS_dot1dTpFdbEntry_ports');
3129

32-
if (!isset($mactrack_scanning_functions_ip)) {
33-
$mactrack_scanning_functions_ip = [];
34-
}
30+
$mactrack_scanning_functions_ip ??= [];
3531
array_push($mactrack_scanning_functions_ip, 'get_cisco_dhcpsnooping_table');
3632
array_push($mactrack_scanning_functions_ip, 'get_cisco_vrf_arp_table');
3733

38-
if (!isset($mactrack_scanning_functions_dot1x)) {
39-
$mactrack_scanning_functions_dot1x = [];
40-
}
34+
$mactrack_scanning_functions_dot1x ??= [];
4135
array_push($mactrack_scanning_functions_dot1x, 'get_cisco_dot1x_table');
4236

4337
/* get_catalyst_doet1dTpFdbEntry_ports

‎lib/mactrack_dell.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,7 @@
2323
*/
2424

2525
// register this functions scanning functions
26-
if (!isset($mactrack_scanning_functions)) {
27-
$mactrack_scanning_functions = [];
28-
}
26+
$mactrack_scanning_functions ??= [];
2927
array_push($mactrack_scanning_functions, 'get_dell_dot1q_switch_ports');
3028

3129
/* get_dell_dot1q_switch_ports - This is a basic function that will scan the dot1d

‎lib/mactrack_dlink.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,10 @@
2424
*/
2525

2626
// register this functions scanning functions
27-
if (!isset($mactrack_scanning_functions)) {
28-
$mactrack_scanning_functions = [];
29-
}
27+
$mactrack_scanning_functions ??= [];
3028
array_push($mactrack_scanning_functions, 'get_dlink_l2_switch_ports');
3129

32-
if (!isset($mactrack_scanning_functions)) {
33-
$mactrack_scanning_functions = [];
34-
}
30+
$mactrack_scanning_functions ??= [];
3531
array_push($mactrack_scanning_functions, 'get_dlink_l2_dot1dTpFdbEntry_ports');
3632

3733
/* get_generic_switch_ports - This is a basic function that will scan the dot1d

‎lib/mactrack_enterasys.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@
2424
*/
2525

2626
// register this functions scanning functions
27-
if (!isset($mactrack_scanning_functions)) {
28-
$mactrack_scanning_functions = [];
29-
}
27+
$mactrack_scanning_functions ??= [];
3028
array_push($mactrack_scanning_functions, 'get_enterasys_switch_ports');
3129

3230
function get_enterasys_switch_ports($site, &$device, $lowPort = 0, $highPort = 0) {

‎lib/mactrack_enterasys_N7.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,10 @@
2323
*/
2424

2525
// register this functions scanning functions
26-
if (!isset($mactrack_scanning_functions)) {
27-
$mactrack_scanning_functions = [];
28-
}
26+
$mactrack_scanning_functions ??= [];
2927
array_push($mactrack_scanning_functions, 'get_enterasys_N7_switch_ports');
3028

31-
if (!isset($mactrack_scanning_functions_ip)) {
32-
$mactrack_scanning_functions_ip = [];
33-
}
29+
$mactrack_scanning_functions_ip ??= [];
3430
array_push($mactrack_scanning_functions_ip, 'get_CTAlias_table');
3531

3632
/* get_generic_switch_ports - This is a basic function that will scan the dot1d
@@ -298,7 +294,7 @@ function get_enterasys_N7_dot1dTpFdbEntry_ports($site, &$device, &$ifInterfaces,
298294

299295
// now set the real data
300296
$new_port_key_array[$i]['key'] = mactrack_arr_key($port_key, 'key');
301-
$new_port_key_array[$i]['port_number'] = isset($brPortIfIndex) ? $brPortIfIndex : '';
297+
$new_port_key_array[$i]['port_number'] = $brPortIfIndex ?? '';
302298
$new_port_key_array[$i]['vlan_id'] = mactrack_arr_key($vlan_ids, $port_key['key']);
303299
// print_r($new_port_key_array[$i]);
304300
$i++;

0 commit comments

Comments
 (0)