Skip to content

fix(security): defense-in-depth hardening for plugin_mactrack#325

Merged
TheWitness merged 6 commits intoCacti:developfrom
somethingwithproof:fix/defense-in-depth
Apr 24, 2026
Merged

fix(security): defense-in-depth hardening for plugin_mactrack#325
TheWitness merged 6 commits intoCacti:developfrom
somethingwithproof:fix/defense-in-depth

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

Summary

Automated defense-in-depth hardening addressing 76 security audit findings.

  • XSS: Escape request variables in HTML value attributes with html_escape_request_var()
  • SQLi: Convert string-concatenated queries to prepared statements
  • Deserialization: Add allowed_classes => false to unserialize() calls
  • Temp files: Replace predictable rand() with tempnam()

All changes are PHP 7.0+ compatible for Cacti 1.2.x.

Test plan

  • PHP lint clean on all changed files
  • Verify plugin functionality after changes

Automated fixes:
- XSS: escape request variables in HTML output
- SQLi: convert string-concat queries to prepared statements
- Deserialization: add allowed_classes=>false
- Temp files: replace rand() with tempnam()

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Copilot AI review requested due to automatic review settings April 9, 2026 06:09
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

Defense-in-depth security hardening for the plugin_mactrack codebase by reducing XSS exposure in UI filters and tightening PHP deserialization behavior.

Changes:

  • Hardened multiple unserialize() call sites by adding allowed_classes => false.
  • Escaped request variables used inside HTML value attributes via html_escape_request_var().
  • Updated Net_DNS2 cache loaders to use hardened unserialization when reading persisted cache state.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Net/DNS2/Cache/Shm.php Adds allowed_classes => false when unserializing shared-memory cache data.
Net/DNS2/Cache/File.php Adds allowed_classes => false when unserializing file cache data.
Net/DNS2/Cache.php Adds allowed_classes => false when unserializing cached response objects.
mactrack_view_macs.php Hardens deserialization of selected_items; escapes filter input value.
mactrack_view_ips.php Escapes hidden page request variable in form.
mactrack_view_interfaces.php Escapes filter request variable in input value attribute.
mactrack_view_devices.php Escapes filter and hidden page request variables in form.
mactrack_view_arp.php Escapes filter request variable in input value attribute.
mactrack_vendormacs.php Escapes filter request variable in input value attribute.
mactrack_snmp.php Escapes filter and hidden page request variables in form.
mactrack_macwatch.php Escapes filter and hidden page request variables in form.
mactrack_macauth.php Escapes filter and hidden page request variables in form.
mactrack_devices.php Escapes filter request variable in input value attribute.
mactrack_device_types.php Escapes filter request variable in input value attribute.
lib/mactrack_functions.php Escapes filter request variable in site filter input value attribute.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread mactrack_view_macs.php Outdated
Comment thread Net/DNS2/Cache.php
Replace .click(fn) with .on('click', fn), .change(fn) with
.on('change', fn), .submit(fn) with .on('submit', fn), .unbind()
with .off(), and .resize(fn) with .on('resize', fn).

These shorthands were deprecated in jQuery 3.3 and will be removed
in jQuery 4.0. Cacti core ships jQuery 3.x on develop.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
- Change Dependabot ecosystem from npm to composer (PHP-only repo)
- Remove PHP from CodeQL paths-ignore so security PRs get analysis
- Remove committed .omc session artifacts, add .omc/ to .gitignore

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof marked this pull request as draft April 11, 2026 00:09
@somethingwithproof
Copy link
Copy Markdown
Contributor Author

Converted to draft to serialize the stack in this repo. Blocked by #324; will un-draft after that merges to avoid cross-PR merge conflicts.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
TheWitness
TheWitness previously approved these changes Apr 24, 2026
xmacan
xmacan previously approved these changes Apr 24, 2026
@TheWitness TheWitness requested review from TheWitness and xmacan April 24, 2026 13:51
@TheWitness TheWitness marked this pull request as ready for review April 24, 2026 14:04
@TheWitness TheWitness merged commit 87d92a1 into Cacti:develop Apr 24, 2026
4 checks passed
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.

4 participants