hardening: parameterize domain stripping update query#274
hardening: parameterize domain stripping update query#274somethingwithproof wants to merge 2 commits intoCacti:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens the syslog_strip_incoming_domains() function in functions.php by replacing interpolated SQL variables with a prepared statement, addressing the SQL injection surface identified in issue #261. It also adds input validation (trimming and skipping empty domain tokens) and a regression test to prevent re-introduction of the interpolated query pattern.
Changes:
- Replaced
syslog_db_execute()withsyslog_db_execute_prepared()insyslog_strip_incoming_domains(), parameterizing the$domainand$uniqueIDvalues - Added input sanitization (trim + empty check) for domain tokens before executing the SQL update
- Added a regression test that verifies the absence of legacy interpolated patterns and presence of parameterized patterns in
functions.php
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| functions.php | Converted domain stripping query from interpolated SQL to prepared statement with input validation |
| tests/regression/issue261_domain_strip_parameterized_test.php | New regression test verifying parameterized query patterns in functions.php |
| CHANGELOG.md | Added entry for issue #261 |
functions.php
Outdated
| } | ||
|
|
||
| syslog_db_execute_prepared('UPDATE `' . $syslogdb_default . '`.`syslog_incoming` | ||
| SET host = SUBSTRING_INDEX(host, ".", 1) |
There was a problem hiding this comment.
The dot delimiter is wrapped in double quotes (".") rather than single quotes. In standard SQL, string literals should be enclosed in single quotes; double quotes are reserved for identifiers. While MySQL's default mode treats both interchangeably, this could break if ANSI_QUOTES SQL mode is ever enabled, where "." would be interpreted as a column identifier instead of a string literal. Consider using escaped single quotes within the PHP string (e.g., '..., \'.\', ...') to remain SQL-standard compliant and consistent with the original code that used '.'.
| SET host = SUBSTRING_INDEX(host, ".", 1) | |
| SET host = SUBSTRING_INDEX(host, \'.\', 1) |
Fixes Cacti#261 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
ANSI_QUOTES mode treats double-quoted identifiers as column/table names, not string literals. Switch to single quotes so the delimiter '.' is unambiguously a string on any sql_mode setting. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
force-pushed
the
hardening/parameterize-domain-strip-update
branch
from
3a8cae0 to
fadf718
Compare
Summary
Fixes #261
Validation