fix(security): defense-in-depth hardening for plugin_flowview#250
fix(security): defense-in-depth hardening for plugin_flowview#250somethingwithproof wants to merge 7 commits intoCacti:developfrom
Conversation
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>
There was a problem hiding this comment.
Pull request overview
Defense-in-depth hardening for the Flowview plugin by tightening request handling in UI links and reducing PHP object injection risk in DNS cache deserialization, plus introducing automated dependency update configuration.
Changes:
- Restrict
unserialize()behavior in Net_DNS2 caching code viaallowed_classes => false. - Sanitize/coerce
idrequest parameters to integers when interpolating into URLs/JS in Flowview UI pages. - Add a Dependabot configuration for GitHub Actions (and npm).
Reviewed changes
Copilot reviewed 6 out of 6 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 metadata. |
Net/DNS2/Cache/File.php |
Adds allowed_classes => false when unserializing file cache metadata. |
Net/DNS2/Cache.php |
Adds allowed_classes => false when unserializing cached DNS response objects. |
flowview_schedules.php |
Coerces id to int via get_filter_request_var() for safer URL/JS interpolation. |
flowview_devices.php |
Coerces id to int via get_filter_request_var() for safer URL interpolation. |
.github/dependabot.yml |
Introduces Dependabot update schedules for npm and GitHub Actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ($this->cache_serializer == 'json') { | ||
| return json_decode($this->cache_data[$key]['object']); | ||
| } else { | ||
| return unserialize($this->cache_data[$key]['object']); | ||
| return unserialize($this->cache_data[$key]['object'], array('allowed_classes' => false)); | ||
| } |
There was a problem hiding this comment.
unserialize(..., ['allowed_classes' => false]) will prevent cached DNS response objects from being rehydrated as their original Net_DNS2_* classes when cache_serializer is serialize (it will return __PHP_Incomplete_Class trees). That breaks the documented behavior that serialize mode preserves class info and can break consumers relying on instanceof or methods. Consider either (a) switching cache serialization to a safe non-object format (e.g., JSON) when caching is enabled, or (b) using an explicit allowlist of the specific Net_DNS2_* classes that can appear in cached responses instead of false.
.github/dependabot.yml
Outdated
| - package-ecosystem: "npm" | ||
| directory: "/" | ||
| schedule: | ||
| interval: "weekly" | ||
| open-pull-requests-limit: 10 |
There was a problem hiding this comment.
This Dependabot config enables the npm ecosystem in /, but the repo does not contain a package.json (or lockfile) at the root. That will cause Dependabot update jobs to fail. Either remove the npm entry or point it at the directory that actually contains package.json (and commit the manifest if it’s intended to exist).
| - package-ecosystem: "npm" | |
| directory: "/" | |
| schedule: | |
| interval: "weekly" | |
| open-pull-requests-limit: 10 |
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>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Summary
Automated defense-in-depth hardening addressing 53 security audit findings.
html_escape_request_var()allowed_classes => falsetounserialize()callsrand()withtempnam()All changes are PHP 7.0+ compatible for Cacti 1.2.x.
Test plan