fix(security): parameterize untrusted SQL inputs across develop#7210
Open
somethingwithproof wants to merge 1 commit into
Open
fix(security): parameterize untrusted SQL inputs across develop#7210somethingwithproof wants to merge 1 commit into
somethingwithproof wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR hardens several request-driven SQL/query construction paths by sanitizing ID lists, properly quoting user-provided regex/search strings, and adding validation for report regex inputs.
Changes:
- Sanitizes/unserializes selected item lists and forces integer IDs before using them in
IN (...)clauses. - Replaces string-concatenated regex/search predicates with
db_qstr(...)quoting in multiple query builders. - Adds regex validation to the reports form save flow and tightens sorting parameter handling in automation views.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
managers.php |
Sanitizes selected item input and casts IDs to integers before delete/update operations. |
lib/reports.php |
Quotes REGEXP operands via db_qstr to avoid injection via search/regexp fields. |
lib/html_tree.php |
Tightens template multiselect validation regex; quotes RLIKE filters via db_qstr. |
lib/html_reports.php |
Adds server-side validation for graph_name_regexp before saving report items. |
lib/html_graph.php |
Quotes RLIKE filters via db_qstr for graph listing/preview filters. |
lib/html.php |
Casts extracted cg_ graph template IDs to integers. |
lib/api_device.php |
Casts device ID lists to integers before building remote/local purge/delete SQL. |
lib/api_automation.php |
Validates sort column/direction and casts host_template_id to int in SQL predicates. |
data_source_profiles.php |
Casts consolidation function IDs to integers before building NOT IN (...) clauses. |
| db_execute('DELETE FROM poller_reindex WHERE host_id IN (' . implode(', ', $device_ids) . ')', true, $rcnn_id); | ||
| db_execute('DELETE FROM graph_tree_items WHERE host_id IN (' . implode(', ', $device_ids) . ')', true, $rcnn_id); | ||
| db_execute('DELETE FROM reports_items WHERE host_id IN (' . implode(', ', $device_ids) . ')', true, $rcnn_id); | ||
| $int_device_ids = array_map('intval', $device_ids); |
Comment on lines
+361
to
+370
| $save['graph_name_regexp'] = form_input_validate(gnrv('graph_name_regexp'), 'graph_name_regexp', '', true, 3); | ||
|
|
||
| if ($save['graph_name_regexp'] != '') { | ||
| $regex_valid = validate_is_regex($save['graph_name_regexp']); | ||
|
|
||
| if ($regex_valid !== true) { | ||
| $_SESSION[SESS_ERROR_FIELDS]['graph_name_regexp'] = 3; | ||
| raise_message('custom', __('The regular expression "%s" is not valid. Error is %s', htmle($save['graph_name_regexp']), htmle($regex_valid)), MESSAGE_LEVEL_ERROR); | ||
| } | ||
| } |
Comment on lines
+180
to
+182
| $sort_column = api_automation_column_exists(grv('sort_column'), ['host', 'graph_local', 'sites', 'graph_templates', 'graph_templates_graph', 'host_template']) ? grv('sort_column') : 'description'; | ||
| $sort_direction = in_array(strtoupper((string) grv('sort_direction')), ['ASC', 'DESC'], true) ? strtoupper((string) grv('sort_direction')) : 'ASC'; | ||
| $sortby = ($sort_column === 'hostname') ? 'INET_ATON(hostname)' : $sort_column; |
| 'friendly_name' => __('Template'), | ||
| 'filter' => FILTER_VALIDATE_REGEXP, | ||
| 'filter_options' => ['options' => ['regexp' => '(cg_[0-9]|dq_[0-9]|[\-0-9])']], | ||
| 'filter_options' => ['options' => ['regexp' => '^(cg_[0-9]+|dq_[0-9]+|[\-0-9]+)(,(cg_[0-9]+|dq_[0-9]+|[\-0-9]+))*$']], |
Comment on lines
+760
to
+766
| db_execute('DELETE FROM snmpagent_managers WHERE id IN (' . implode(',', array_map('intval', $selected_items)) . ')'); | ||
| db_execute('DELETE FROM snmpagent_managers_notifications WHERE manager_id IN (' . implode(',', array_map('intval', $selected_items)) . ')'); | ||
| db_execute('DELETE FROM snmpagent_notifications_log WHERE manager_id IN (' . implode(',', array_map('intval', $selected_items)) . ')'); | ||
| } elseif (gnrv('drp_action') == '2') { // disable | ||
| db_execute("UPDATE snmpagent_managers SET disabled = 'on' WHERE id IN (" . implode(',' ,$selected_items) . ')'); | ||
| db_execute("UPDATE snmpagent_managers SET disabled = 'on' WHERE id IN (" . implode(',', array_map('intval', $selected_items)) . ')'); | ||
| } elseif (gnrv('drp_action') == '3') { // enable | ||
| db_execute("UPDATE snmpagent_managers SET disabled = '' WHERE id IN (" . implode(',' ,$selected_items) . ')'); | ||
| db_execute("UPDATE snmpagent_managers SET disabled = '' WHERE id IN (" . implode(',', array_map('intval', $selected_items)) . ')'); |
Bind, db_qstr, or integer-cast request-derived values that previously reached SQL by interpolation: RLIKE/REGEXP filters in graph/tree/report views, IN() id lists in device, profile, and SNMP receiver management, host_template_id in automation matching, and ORDER BY columns in the automation match pages (allowlist + ASC/DESC). Validate graph_name_regexp at save and anchor the graph_template_id filter regex. Covers CVE-2026-46531. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
force-pushed
the
fix/develop-sqli-compliance
branch
from
6cd24d0 to
aaacb33
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hardens develop against SQL injection by binding,
db_qstr-quoting, or integer-casting request-derived values that previously reached SQL via interpolation. No functional change for legitimate input.Sites fixed:
lib/html_graph.php,lib/html_tree.php,lib/reports.php) →db_qstr. Covers CVE-2026-46531 and the rfilter/report-regexp class.IN()id lists in device, data-source-profile, and SNMP-receiver management (lib/api_device.php,data_source_profiles.php,managers.php) →array_map('intval', ...); SNMP-receiver path also switched tosanitize_unserialize_selected_items().host_template_idin automation matching (lib/api_automation.php) → int cast.ORDER BYon both automation match pages (hosts + trees) → column allowlist viaapi_automation_column_exists()+ASC/DESCnormalization, mirroring the existing hardened path.graph_name_regexpvalidated at save (lib/html_reports.php);graph_template_idfilter regex anchored (lib/html_tree.php).Verification:
php -lclean on all 9 files.db_qstrpreserves RLIKE/REGEXP semantics,intvalmapping and the anchored regex accept all legitimate inputs (incl. multi-digitcg_12and comma lists), ORDER BY allowlist falls back to a real column.Scope: the SQL-injection class only; output-encoding/XSS, path-containment, redirect, and auth/session passes will follow separately.