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

Cross-site scripting (XSS) vulnerability in aggregate_graphs.php in Cacti 1.1.12 #847

Closed
kimiizhang opened this issue Jul 10, 2017 · 16 comments

Comments

@kimiizhang
Copy link

xiaotian.wang@DBAppSecurity.com.cn

Cross-site scripting (XSS) vulnerability in aggregate_graphs.php in Cacti 1.1.12 allows remote authenticated users to inject arbitrary web script or HTML via specially crafted HTTP Referer headers.

http://192.168.1.206/cacti/aggregate_graphs.php?action=edit&tab=details&id=1
Referer:aaaaaaaaaaaaaaa")'></td></tr><script>alert(1)</script>

11111111

@cigamit
Copy link
Member

cigamit commented Jul 10, 2017

This was fixed here...

#838

@cigamit cigamit closed this as completed Jul 10, 2017
@kimiizhang
Copy link
Author

@cigamit Nope,838 is different from this vul 847, let me show you, your patch for 838 upgrated 'lib/html_validate.php'(3381cba6a9e36b01ed0ab0acfd41b00487966cb5). It's useless for vul 847. To resolve this vul 847, you should change file 'lib/html_form.php', on line 1151:
$cancel_action = "<input type='button' onClick='cactiReturnTo(\"" . $cancel_url . "\")' value='" . $calt . "'>";
the variable '$cancel_url' should be filtered.

Am I clear?

@cigamit
Copy link
Member

cigamit commented Jul 11, 2017

I understand what you are saying, and we can escape that variable, but the design of plugin's that leverage that $cancel_url, should have already been pre-validated by one of the validation/sanitization functions. In your case, for integers, you would validate the variable is clean using the get_filter_request_variable('request_id') function. Had this been done (prior to the other fix), you would have never reached that code block. Now that the other fix it in, you won't.

Now that the 'real' XSS issue has been resolved, you will be unable to reproduce this error. Give it a try. You won't be able to re-produce it. Now, with that said, a plugin developer, if they do not use the validation/sanitization functions, may find themselves compromised by this, and that is a good reason to escape the $cancel_url. But it should not be required if the plugin developer simply follows the rules.

@cigamit
Copy link
Member

cigamit commented Jul 11, 2017

Here is your example using the fix from issue #838

xssissue2

@kimiizhang
Copy link
Author

kimiizhang commented Jul 11, 2017

@cigamit OK, I understand what you said, but in my opinion , this issue 847 is still vulnerable in the latest Cacti version: 1.1.12, isn't it?

Just think that any users who use Cacti now can avoid this issue? Even if with your patch for 838 here:3381cba

As you are Cacti developer, you need to upgrade all affected files, functions, I think you can publish your Cacti 1.1.13 soon to resolve this issue.

@cigamit
Copy link
Member

cigamit commented Jul 11, 2017

If the plugin developers properly validate/sanitize the variables, it is no longer susceptible to this issue.

Additionally, we provide what is called 'Developer Debug' mode that will log all instances of un-validated request variables to help developers find possible explit points in the code. The mistake that was made was that in the validation logic, we echo'd the unsanitized request variable to the browser without using htmlspecialchars() which exposed us to the XSS.

It is imperative that all Cacti developers use the new functions:

  • get_request_var()
  • set_request_var()
  • get_filter_request_var()
  • get_nfilter_request_var()
  • validate_store_request_vars()
  • isset_request_var()
  • isempty_request_var()
  • unset_request_var()

Instead of $_GET, $_REQUEST, $_POST as these functions first check if the variable has been validated/sanitized. The get_nfilter_request_var() function should be used with caution as it is an indication to the Cacti framework that this variable will not be used in raw display to the browser and need not be sanitized (say for example in case, if-then-else logic, etc).

@cigamit
Copy link
Member

cigamit commented Jul 11, 2017

We are additionally planning Wednesday for the 1.1.13 release. There were some last minute fairly major changes to the snmpagent framework that we wish to have out there for people testing for a few days before we release.

@mortenstevens
Copy link
Contributor

@cigamit

Just a small question: For Fedora and Fedora-EPEL I have backported your upstream patch 3381cba to our latest 1.1.2 package. See our downstream commit: https://src.fedoraproject.org/cgit/rpms/cacti.git/commit/?id=d6653ab510eab5ba7b19d4007cca2e7c4e1f4f5c

Is that enough to fix CVE-2017-10970 on cacti 1.1.2?

@cigamit
Copy link
Member

cigamit commented Jul 11, 2017

Yes, though you should consider an update to 1.1.13 once it's released. That'll be the stable version for a while. Once 1.1.13 is released, we will put the 1.1 branch into maintenance mode and start a 1.2 branch where we will begin to work on the 50 or so feature requests that are outstanding and work on doing formal releases of the stable plugins so that, for example, epel can consider RPM packages for plugins based upon a formal release.

@paulgevers
Copy link
Contributor

FYI, this issue got assigned CVE-2017-11163.

@mortenstevens
Copy link
Contributor

@cigamit

Thank you.

@cigamit
@paulgevers

Why is there a new CVE number? I thought there is already CVE-2017-10970 assigned to this issue?

@cigamit
Copy link
Member

cigamit commented Jul 12, 2017

I was asking myself the same question.

cigamit added a commit that referenced this issue Jul 12, 2017
Potential CVE in lib/html_form.php.
@cigamit
Copy link
Member

cigamit commented Jul 12, 2017

I've gone and added the extra protection in case a plugin developer makes a huge mistake. So, we will have this issue marked as officially resolved in the 1.1.13 release.

@paulgevers
Copy link
Contributor

@mortenstevens @cigamit I am the messenger. I don't know. I was told issue #847 got this CVE assigned. I think there are security teams looking at the bug tracker of cacti and requesting CVE's for issues they consider a security issue.

@cigamit
Copy link
Member

cigamit commented Jul 12, 2017

No worries, it was not to get this closed, whether its a real issue or a red herring. This will add extra safeguard in the case that the developer has not followed the rules.

@fgeek
Copy link

fgeek commented Jul 19, 2017

According to the NIST NVD data these CVEs are for different vulnerabilities.

CVE-2017-10970 is for:

Cross-site scripting (XSS) vulnerability in link.php in Cacti 1.1.12 allows remote anonymous users to inject arbitrary web script or HTML via the id parameter, related to the die_html_input_error function in lib/html_validate.php.

CVE-2017-10970 also has #838 URL in the references.

CVE-2017-11163 is for:

Cross-site scripting (XSS) vulnerability in aggregate_graphs.php in Cacti 1.1.12 allows remote authenticated users to inject arbitrary web script or HTML via specially crafted HTTP Referer headers, related to the $cancel_url variable.

CVE-2017-11163 has this issue item in the references.

Are these both fixed in 1.1.13 release? According to http://www.securitytracker.com/id/1038908 they are.

cigamit added a commit that referenced this issue Jul 26, 2017
Improving resolution to #847 and one additional vulnerability.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants