Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2025-24368: protect save_component_automation_tree_rule_item entry point #6094

Closed
wants to merge 1 commit into from

Conversation

Beuc
Copy link
Contributor

@Beuc Beuc commented Feb 6, 2025

Hi,

I'm part of the Debian LTS Team and I'm testing our patch for GHSA-f9c7-7rc3-574c, hence trying to reproduce the vulnerability.

c7e4ee7 is the current Cacti official fix (+ 94526a9).

It seems that automation_tree_rules.php now protects the save_component_automation_match_item entry point, rather than the save_component_automation_tree_rule_item described in the GHSA.
This is also inconsistent with the automation_graph_rules.php fix which protects the save_component_automation_graph_rule_item entry point.

No directly usable PoC was published, so I can't guarantee it, but the following patch should be more consistent with the GHSA, and actually triggers when I try to recreate the payload described there (in particular 'save_component_automation_tree_rule_item' => 1).

Please let me know if I'm mistaken :)

@TheWitness
Copy link
Member

It certainly belongs in the original position. However, there are two sections that involve query fields, and there was a reason that I did not do the section that you moved the check to in the later case and this is due primarily to the way that the column is used during the automation. I'm digging in. We already had some snafu's with the release that need to be fixed. I'll make this double check today, and also give you some reproduce steps to test the exploit on the Tree automation as well.

@TheWitness
Copy link
Member

Okay, we could place the check in that second location. But it looks like what Cacti is doing in the automation is catching this on the backend process. If you look at lib/api_automation.php line 1189, you can see where the attempted pollution of the input stream is captured. I can add the SQL injection front end check too, and I guess it would be nice if it were more consistent. What do you think?

@TheWitness
Copy link
Member

Digging deeper now. It looks like the placement in automation_graph_rules.php was incorrect. So, we have backend checks, so the CVE is still addressed, but we need to still address the issues on the frontend.

@TheWitness
Copy link
Member

Note the changes here.

#6095

@Beuc
Copy link
Contributor Author

Beuc commented Feb 7, 2025

Thanks for the clarifications :)
If I understand correctly:

For the backend (actual SQLI), this vulnerability was really fixed in 1.2.27 (8b516cb / CVE-2024-31460).

For the front-end ("store dirty data" step in GHSA-f9c7-7rc3-574c), this is not a vulnerability in itself.
And, if we still want to check for suspicious changes, it seems we'd need checks in at least both save_component_automation_*_rule_item and save_component_automation_match_item, as either entry point could store arbitrary data in the field field in the database.
Which now appears to be the case in #6095 / e1d0450 for automation_graph_rules.php, though not for automation_tree_rules.php.

Given that the core vulnerability is covered in the backend, I'm not sure there's need for more.

@TheWitness
Copy link
Member

Thanks for keeping me honest!

@TheWitness TheWitness closed this Feb 7, 2025
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