Skip to content

fix(security): defense-in-depth hardening for plugin_wmi#9

Draft
somethingwithproof wants to merge 2 commits intoCacti:developfrom
somethingwithproof:fix/defense-in-depth
Draft

fix(security): defense-in-depth hardening for plugin_wmi#9
somethingwithproof wants to merge 2 commits intoCacti:developfrom
somethingwithproof:fix/defense-in-depth

Conversation

@somethingwithproof
Copy link
Copy Markdown

Summary

Defense-in-depth hardening addressing 31 security audit findings.

  • XSS: Escape request variables in HTML output with html_escape_request_var()
  • SQLi: Convert string-concat queries to prepared statements
  • Deserialization: Add allowed_classes => false to unserialize()

All changes PHP 7.0+ compatible.

Test plan

  • PHP lint clean
  • Pre-push review PASS (Claude + Grok + Copilot)
  • Verify plugin functionality

Automated fixes:
- XSS: escape request variables in HTML value attributes
- 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:16
Copy link
Copy Markdown

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 the plugin_wmi Linux WMI implementation against PHP object injection by restricting unserialize() to disallow classes when decoding stored account credentials.

Changes:

  • Add allowed_classes => false to unserialize() in credential decode path.

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

Comment thread linux_wmi.php
Comment on lines 291 to 293
$info = base64_decode($info);
$info = unserialize($info);
$info = unserialize($info, array('allowed_classes' => false));
$info = $info['password'];
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.

unserialize() (even with allowed_classes => false) and base64_decode() can return false on malformed/corrupt data. Immediately indexing $info['password'] will raise warnings/notices and may break authentication flow. Consider validating that the decoded/unserialized value is an array with the expected key (and returning a safe failure / setting $this->error) when decoding fails.

Copilot uses AI. Check for mistakes.
- 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:10
@somethingwithproof
Copy link
Copy Markdown
Author

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

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.

2 participants