Skip to content

security: parameterize SQL in sequence functions and data_queries.php (1.2.x backport)#6911

Merged
TheWitness merged 3 commits intoCacti:1.2.xfrom
somethingwithproof:backport/1.2.x-sqli-sequence-functions
Mar 29, 2026
Merged

security: parameterize SQL in sequence functions and data_queries.php (1.2.x backport)#6911
TheWitness merged 3 commits intoCacti:1.2.xfrom
somethingwithproof:backport/1.2.x-sqli-sequence-functions

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

Summary

Surgical edits to current 1.2.x files. +139/-11 lines.

  • Add build_where_from_array() helper for safe parameterized WHERE clauses
  • Update get_item() and get_sequence() to accept array filters (backward compatible with string)
  • Convert 4 raw SQL callers in data_queries.php to use parameterized arrays
  • Replace db_fetch_cell() with db_fetch_cell_prepared() in get_item
  • Replace db_fetch_row() with db_fetch_row_prepared() in get_sequence
  • Add unit tests

Security

  • GHSA-w839-f83f-fj47 (Critical) - SQL Injection in data_queries.php
  • GHSA-pf37-v86f-5xwp (High) - Stored SQL Injection via graph_name_regexp in Reports
  • GHSA-9f3h-m984-g777 (High) - Second-Order SQL Injection via graph_name_regexp

Test plan

  • Verify data query graph/data source sorting works
  • Verify sequence ordering in SNMP query graph items
  • Run vendor/bin/pest tests/Unit/DbLayerHardeningTest.php

Copilot AI review requested due to automatic review settings March 28, 2026 23:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Hardens Cacti 1.2.x sequence/move helpers and data_queries.php by adding a safe, parameterized way to construct simple WHERE clauses and migrating select callers away from raw SQL concatenation to prepared queries to mitigate SQL injection risks.

Changes:

  • Added build_where_from_array() and extended get_item() / get_sequence() to accept array-based filters (in addition to legacy string WHERE fragments).
  • Updated get_item() / get_sequence() to use prepared DB helpers (db_fetch_cell_prepared, db_fetch_row_prepared) for safer execution.
  • Converted several data_queries.php move-up/down callers to pass array filters instead of concatenated SQL.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
lib/functions.php Adds array-to-WHERE helper and updates sequence/item retrieval to use prepared queries and accept array filters.
data_queries.php Switches several move-up/down calls to pass structured filters (arrays) rather than interpolated SQL.
tests/Unit/DbLayerHardeningTest.php Introduces a new unit test file intended to validate the new WHERE-builder behavior.

TheWitness
TheWitness previously approved these changes Mar 28, 2026
@TheWitness
Copy link
Copy Markdown
Member

This one looks correct.

… (1.2.x backport)

- Add build_where_from_array() helper for safe parameterized WHERE clauses
- Update get_item() and get_sequence() to accept array filters
- Convert 4 raw SQL callers in data_queries.php to use parameterized arrays
- Add unit tests for DB layer hardening

Addresses GHSA-w839-f83f-fj47 (Critical), GHSA-pf37-v86f-5xwp (High), GHSA-9f3h-m984-g777 (High)

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@TheWitness TheWitness requested review from bmfmancini and xmacan March 29, 2026 13:48
@TheWitness TheWitness merged commit 0892600 into Cacti:1.2.x Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants