Skip to content

(hardening) Protect QuickTree add and clear actions against CSRF - #42

Open
somethingwithproof wants to merge 3 commits into
mainfrom
fix/hardening-quicktree-csrf-40
Open

somethingwithproof wants to merge 3 commits into
mainfrom
fix/hardening-quicktree-csrf-40

Conversation

@somethingwithproof

Copy link
Copy Markdown
Member

Adding or clearing queued graphs currently changes state through GET, including the bulk-action clear alias. Validate the effective action after remapping, require POST and a successful Cacti CSRF check, and send graph additions as JSON-response AJAX POST requests through the host CSRF-aware transport. The existing bulk clear form already submits by POST.

Validation: PHP 8.4.25 standalone regression, 16 direct/alias request cases passed; Node 22.22.2 client regression passed, including preserved graph fields and notifications; syntax and diff checks passed.

Closes #40.

@somethingwithproof somethingwithproof self-assigned this Oct 10, 2026
@somethingwithproof
somethingwithproof force-pushed the fix/hardening-quicktree-csrf-40 branch from c0e3160 to d43e106 Compare October 17, 2026 19:46
@somethingwithproof
somethingwithproof changed the base branch from develop to main October 17, 2026 19:46
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

@TheWitness TheWitness left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We keep doing this differently. I'm not entirely happy with the way it's done here.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof

Copy link
Copy Markdown
Member Author

778a07d routes the add and clear checks through one named guard in includes/security.php. Which plugin or pattern should this match?

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@sonarqubecloud

Copy link
Copy Markdown

@somethingwithproof

Copy link
Copy Markdown
Member Author

Resolved the conflict with main in 6ed16a1, preserving both the named CSRF guard and the newly merged tree-authorization helper. The 23 mutation cases, four authorization cases and JavaScript POST test pass locally; hosted PHP 8.2, 8.3 and 8.4 integration and Sonar checks also pass. The requested consistency review remains open for your decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: add and clear mutate QuickTree state through unprotected GET requests

2 participants