Skip to content

Commit 11b0d02

Browse files
fix: resolve SQL injection and correctness issues
- Fix SQL injection in rfilter RLIKE expression by escaping with db_qstr() - Add missing JOIN ON clause in GROUP_CONCAT query (syntax error) - Initialize $host_ids array before loop to prevent undefined variable errors - Move sort() outside foreach loop for O(n log n) instead of O(n² log n) - Fix JavaScript comparison: change 'action >= ""' to 'action !== ""' - Fix tooltip positioning typo: '1eft' -> 'left' - Fix spelling: 'tropper' -> 'stormtrooper' Addresses review feedback from PR Cacti#202. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
1 parent 258942e commit 11b0d02

3 files changed

Lines changed: 12 additions & 8 deletions

File tree

db_functions.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ function renderWhereJoin(string &$sql_where, string &$sql_join): void
114114
}
115115

116116
if (get_request_var('rfilter') != '') {
117-
$awhere .= ($awhere == '' ? '' : ' AND ') . " h.description RLIKE '" . get_request_var('rfilter') . "'";
117+
$awhere .= ($awhere == '' ? '' : ' AND ') . " h.description RLIKE '" . db_qstr(get_request_var('rfilter')) . "'";
118118
}
119119

120120
if (get_request_var('grouping') == 'tree') {
@@ -230,6 +230,7 @@ function getHostsDownOrTriggeredByPermission(bool $prescan): array
230230
'SELECT GROUP_CONCAT(DISTINCT host_id) AS hosts
231231
FROM graph_tree_items AS gti
232232
INNER JOIN host AS h
233+
ON h.id = gti.host_id
233234
WHERE host_id > 0
234235
AND h.deleted = ""
235236
AND graph_tree_id = ?',
@@ -263,7 +264,7 @@ function getHostsDownOrTriggeredByPermission(bool $prescan): array
263264
);
264265
}
265266

266-
$sql_where = "h.monitor = 'on'
267+
$sql_where = "h.monitor = 'on'
267268
AND h.disabled = ''
268269
AND h.deleted = ''
269270
AND ((h.status < " . $PreScanValue . ' AND (h.availability_method > 0 OR h.snmp_version > 0)) ' .
@@ -275,8 +276,8 @@ function getHostsDownOrTriggeredByPermission(bool $prescan): array
275276
if (cacti_sizeof($hosts)) {
276277
foreach ($hosts as $host) {
277278
$result[] = $host['id'];
278-
sort($result);
279279
}
280+
sort($result);
280281
}
281282

282283
return $result;

js/monitor.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ function timeStep() {
5959
} else {
6060
$('#timer').html(value);
6161
// What is a second, well if you are an
62-
// imperial storm tropper, it's just a little more than a second.
62+
// imperial stormtrooper, it's just a little more than a second.
6363
myTimer = setTimeout(timeStep, 1284);
6464
}
6565
}
@@ -92,7 +92,7 @@ function applyFilter(action = '') {
9292
} else {
9393
strURL = 'monitor.php?header=false';
9494

95-
if (action >= '') {
95+
if (action !== '') {
9696
strURL += `&action=${action}`;
9797
}
9898

@@ -341,7 +341,7 @@ $(() => {
341341
myTimer = setTimeout(timeStep, 1000);
342342

343343
$(globalThis).resize(() => {
344-
$(document).tooltip('option', 'position', { my: '1eft:15 top', at: 'right center' });
344+
$(document).tooltip('option', 'position', { my: 'left:15 top', at: 'right center' });
345345
});
346346

347347
if ($('#mute').val() === 'true') {

monitor_render.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,7 @@ function renderSite(): string
185185
$suppressGroups = true;
186186
}
187187

188+
$host_ids = [];
188189
foreach ($hosts as $index => $host) {
189190
if (is_device_allowed($host['id'])) {
190191
$host_ids[] = $host['id'];
@@ -318,6 +319,7 @@ function renderTemplate(): string
318319
$suppressGroups = true;
319320
}
320321

322+
$host_ids = [];
321323
foreach ($hosts as $index => $host) {
322324
if (is_device_allowed($host['id'])) {
323325
$host_ids[] = $host['id'];
@@ -915,7 +917,8 @@ function renderHeaderList(int $total_rows = 0, int $rows = 0): string
915917
'hostname' => [
916918
'display' => __('Hostname', 'monitor'),
917919
'sort' => 'ASC',
918-
'align' => 'left', 'tip' => __('Hostname of device', 'monitor')
920+
'align' => 'left',
921+
'tip' => __('Hostname of device', 'monitor')
919922
],
920923
'id' => [
921924
'display' => __('ID', 'monitor'),
@@ -1112,7 +1115,7 @@ function renderHostList(array $host): string
11121115
}
11131116

11141117
if ($host['availability_method'] > 0) {
1115-
$host_avg = __('%d ms', $host['cur_time'], 'monitor') . ' / ' . __('%d ms', $host['avg_time'], 'monitor');
1118+
$host_avg = __('%d ms', $host['cur_time'], 'monitor') . ' / ' . __('%d ms', $host['avg_time'], 'monitor');
11161119
} else {
11171120
$host_avg = __('N/A', 'monitor');
11181121
}

0 commit comments

Comments
 (0)