Skip to content

fix(hardening): migrate RLIKE to db_qstr for SQL injection prevention #764

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

Closed
somethingwithproof wants to merge 1 commit into from
Closed

fix(hardening): migrate RLIKE to db_qstr for SQL injection prevention #764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions notify_lists.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1399,7 +1399,7 @@ function tholds($header_label) {
}

if (strlen(get_request_var('rfilter'))) {
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . "td.name_cache RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . "td.name_cache RLIKE " . db_qstr(get_request_var('rfilter')) . "";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The final . "" concatenation is redundant here. Removing it would improve readability (db_qstr already returns a complete quoted SQL literal).

Suggested change
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . "td.name_cache RLIKE " . db_qstr(get_request_var('rfilter')) . "";
$sql_where .= (!strlen($sql_where) ? '' : ' AND ') . 'td.name_cache RLIKE ' . db_qstr(get_request_var('rfilter'));

Copilot uses AI. Check for mistakes.
}

if ($statefilter != '') {
Expand Down Expand Up @@ -1739,7 +1739,7 @@ function templates($header_label) {
}

if (strlen(get_request_var('rfilter'))) {
$sql_where .= (!strlen($sql_where) ? 'WHERE ' : ' AND ') . "thold_template.name RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= (!strlen($sql_where) ? 'WHERE ' : ' AND ') . "thold_template.name RLIKE " . db_qstr(get_request_var('rfilter')) . "";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

This SQL fragment ends with a no-op . "" concatenation. Consider removing it to keep the clause assembly straightforward and consistent.

Suggested change
$sql_where .= (!strlen($sql_where) ? 'WHERE ' : ' AND ') . "thold_template.name RLIKE " . db_qstr(get_request_var('rfilter')) . "";
$sql_where .= (!strlen($sql_where) ? 'WHERE ' : ' AND ') . "thold_template.name RLIKE " . db_qstr(get_request_var('rfilter'));

Copilot uses AI. Check for mistakes.
}

$sql = "SELECT *
Expand Down Expand Up @@ -2144,9 +2144,9 @@ function clearFilter() {
// form the 'where' clause for our main sql query
if (strlen(get_request_var('rfilter'))) {
$sql_where = "WHERE (
name RLIKE '" . get_request_var('rfilter') . "'
OR description RLIKE '" . get_request_var('rfilter') . "'
OR emails RLIKE '" . get_request_var('rfilter') . "')";
name RLIKE " . db_qstr(get_request_var('rfilter')) . "
OR description RLIKE " . db_qstr(get_request_var('rfilter')) . "
OR emails RLIKE " . db_qstr(get_request_var('rfilter')) . ")";
} else {
$sql_where = '';
}
Expand Down
2 changes: 1 addition & 1 deletion thold.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,7 @@ function list_tholds() {
}

if (get_request_var('rfilter') != '') {
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE " . db_qstr(get_request_var('rfilter')) . "";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The trailing string concatenation (. "") is redundant here and slightly obscures the intent of the SQL fragment. Consider removing the final . "" (and the leading space before td.name_cache if not needed) to keep the clause construction consistent and easier to read.

Suggested change
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE " . db_qstr(get_request_var('rfilter')) . "";
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . 'td.name_cache RLIKE ' . db_qstr(get_request_var('rfilter'));

Copilot uses AI. Check for mistakes.
}

if ($statefilter != '') {
Expand Down
10 changes: 5 additions & 5 deletions thold_graph.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ function tholds() {
$statefilter = thold_get_state_filter(get_request_var('state'));

if (get_request_var('rfilter') != '') {
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE " . db_qstr(get_request_var('rfilter')) . "";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The trailing . "" concatenation is unnecessary and can be removed to simplify this SQL clause construction (db_qstr already returns the fully quoted value).

Suggested change
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE " . db_qstr(get_request_var('rfilter')) . "";
$sql_where .= ($sql_where == '' ? '(' : ' AND ') . " td.name_cache RLIKE " . db_qstr(get_request_var('rfilter'));

Copilot uses AI. Check for mistakes.
}

if (get_request_var('data_template_id') != '-1') {
Expand Down Expand Up @@ -937,8 +937,8 @@ function hosts() {

if (get_request_var('rfilter') != '') {
$sql_where .= " (h.deleted = ''
AND (h.hostname RLIKE '" . get_request_var('rfilter') . "'
OR h.description RLIKE '" . get_request_var('rfilter') . "')";
AND (h.hostname RLIKE " . db_qstr(get_request_var('rfilter')) . "
OR h.description RLIKE " . db_qstr(get_request_var('rfilter')) . ")";
}

if (get_request_var('host_status') == '-1') {
Expand Down Expand Up @@ -1395,7 +1395,7 @@ function thold_export_log() {
}

if (get_request_var('rfilter') != '') {
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE " . db_qstr(get_request_var('rfilter')) . "";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The trailing . "" concatenation at the end of this expression is redundant. Removing it will make the WHERE clause assembly clearer without changing behavior.

Suggested change
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE " . db_qstr(get_request_var('rfilter')) . "";
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE " . db_qstr(get_request_var('rfilter'));

Copilot uses AI. Check for mistakes.
}

$sql_order = '';
Expand Down Expand Up @@ -1490,7 +1490,7 @@ function thold_show_log() {
}

if (get_request_var('rfilter') != '') {
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE '" . get_request_var('rfilter') . "'";
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE " . db_qstr(get_request_var('rfilter')) . "";
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

This expression ends with a redundant . "" concatenation. Consider dropping it to keep the SQL filter construction concise and consistent with the surrounding code.

Suggested change
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE " . db_qstr(get_request_var('rfilter')) . "";
$sql_where .= ($sql_where == '' ? '' : ' AND') . " tl.description RLIKE " . db_qstr(get_request_var('rfilter'));

Copilot uses AI. Check for mistakes.
}

$sql_order = get_order_string();
Expand Down
Loading