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-2009-4112 still exists (after 8 years) #1072

Closed
paulgevers opened this issue Nov 10, 2017 · 30 comments
Closed

CVE-2009-4112 still exists (after 8 years) #1072

paulgevers opened this issue Nov 10, 2017 · 30 comments
Assignees
Labels
bug Undesired behaviour enhancement General tag for an enhancement
Milestone

Comments

@paulgevers
Copy link
Contributor

Due to the migration of cacti development to github, one old bug that I care a bit about got lost.

CVE-2009-4112¹ still exists:

Cacti 0.8.7e and earlier allows remote authenticated administrators to gain privileges by modifying the "Data Input Method" for the "Linux - Get Memory Usage" setting to contain arbitrary commands.

The issue is of course broader, as I can create a new Data Input Method, I don't need to change an existing one.

I find suggestions² that at the time the item was reported that cacti developers (at least one) were considering implementing a whitelist as a solution. I also find suggestions³ that this issue would be considered during design of 0.8.8, but that ship has sailed long time ago.

I do recognize that fixing this issue is difficult due to the design and philosophy of Data Input Methods. But maybe an (optional) whitelist could be implemented to only allow commands that are stored by the system administrator (via a configuration file on the file system). I.e. if the file is there, whitelisting is effective, and if it is not, than all commands are allowed.

The reason why I post this issue is because of recent other security issues (#1057 and #1066) in this tracker. It seems that as long as this old issue isn't fixed, fixing the other issues is not really improving security.

¹ https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-4112
² https://security-tracker.debian.org/tracker/CVE-2009-4112 (in the notes)
³ http://www.securityfocus.com/archive/1/archive/1/508129/100/0/threaded (bottom of the page)

@paulgevers paulgevers changed the title CVE-2009-4112 still exist (after 8 years) CVE-2009-4112 still exists (after 8 years) Nov 10, 2017
@cigamit
Copy link
Member

cigamit commented Nov 11, 2017

Paul, I've discussed this with Tony and he has asked for some time to respond. I have proposed a solution that can be optional for people who are concerned about this vector. As you can imagine, this is at the very edge of what Tony referred to as "laser focused spear pfishing", with a very low probability of success. However, as we have to support a very large community of users, we will get this thing finally addressed.

I'm not certain you will get a response this weekend. But hey this has been around for 8 years.

@paulgevers
Copy link
Contributor Author

@cigamit I am not in a rush at all :) And I agree you certainly want to think this over and don't implement anything to hastily. It isn't worth it (haste).

cigamit added a commit that referenced this issue Nov 12, 2017
CVE-2017-16785 in global_session.php Reflection XSS issue
@cigamit
Copy link
Member

cigamit commented Nov 12, 2017

Sorry, wrong issue number should have been resolving issue #1071

@cigamit
Copy link
Member

cigamit commented Nov 21, 2017

Here is the proposal Paul. We will set a config option that points to a white list file in a location selected by the packager (my guess /etc/cacti). If that setting is present, then cacti will behave as documented below. Otherwise, it will behave as it does today.

First, the white list file will include name value pairs as follows:

  1. The hash of each Data Input Method.
  2. The the base64 encoded value of the Data Input Methods command/input string.

If at any time, during Graph/Data Source creation, poller cache re-population, etc, the following is true:

  1. The base64 encoded value does not match the value in the database
  2. There is no entry in the white list file matching the hash of the input method.

Data sources will not be permitted to be created, the poller item table will remain touched, etc.. This will essentially disable the Data Input Method.

Additionally, we will validate the white list file each polling cycle and generate warnings when we find that has issues.

I'm guessing you will need a CLI script to be able to generate and update the white list automatically at install time. So, we will do that as well.

Lastly, we will provide a Utility pick for the Cacti Administrator to re-generate and review the white list contents. However, for security reasons, the Cacti Administrator will have to manually cut and paste the contents into the white list file by hand.

How does that sound?

@paulgevers
Copy link
Contributor Author

@cigamit

First, thanks a lot for coming back to this and spending time on finally fixing this.

Otherwise, it will behave as it does today.

This, I like. :)

I don't see why you need to base64 encode the command, but you'll probably have a reason, so I'm fine with it. Plain text is probably easier to audit by the system administrator, but if there are tools (as you "promise") this is no big deal.

I'm guessing you will need a CLI script to be able to generate and update the white list automatically at install time. So, we will do that as well.

If you mean to migrate an existing setup, that would be awesome and more than I expected to get. To be honest, I was preparing to back-port your changes to existing releases of Cacti in Debian, while not enabling it. So if this CLI script is blocking progress or taking too much time, feel free to leave it out.

How does that sound?

As far as I can tell, sounds good.

cigamit added a commit that referenced this issue Nov 21, 2017
Correcting CVE-2009-4112.  This CVE correction will provide some level
of protection to Cacti when unauthorized changes are made to a Data
Input Method.
@cigamit
Copy link
Member

cigamit commented Nov 21, 2017

Paul, I have committed a fix for this. Please test and provide feedback.

@ronytomen
Copy link
Member

@paulgevers, Please let us know if the last commit works for you we found some issues.

Not sure if you want to backport this to 1.1.x releases, your option, but we are trying to get 1.2.x moving forward and anticipate it early sometime in 2018 (maybe sooner, no promises).

@ronytomen ronytomen added the bug Undesired behaviour label Nov 22, 2017
@ronytomen ronytomen added this to the Cacti Release 1.2 milestone Nov 22, 2017
@paulgevers
Copy link
Contributor Author

@ronytomen I backported the 5 relevant patches to 1.1.28, which only had a conflict in docs/CHANGELOG. I am now running a local 1.1.28 Debian package with those changes applied.

It must be more involved. Because while I now see "White List Verification Succeeded." in the Data Input Method tab, nothing seems to happen when I add the $input_whitelist option to the configuration. Also not when I add the whitelist file, but leaving it empty. I even changed one input method to "touch /tmp/CVE-2009-4112" and I see it appearing on every cron run of Cacti. I see quite some errors in the cacti.log however, so I'll dive into that first.

Sorry, no results from me thus.

@ronytomen
Copy link
Member

Seems to be not working. More testing needed on our side, sorry!

@cigamit
Copy link
Member

cigamit commented Nov 23, 2017

This is my fault. I covered the create case, but not the re-populate case. Easy fix. Be in shortly.

cigamit added a commit that referenced this issue Nov 23, 2017
- Moving the json_decode to associative arrays.
- Add update poller cache validation check that was previously missed
- Minor reformating of function calls, database calls, etc.
@cigamit
Copy link
Member

cigamit commented Nov 23, 2017

Paul, try again. If the whitelist is invalid, the poller items will not be updated to the new values. They will instead be removed from the database until the problem is corrected. I also fixed an issue where half of the code was using the old whitelist format, and the rest was using the new. Move things to associative arrays from objects for consistency.

@paulgevers
Copy link
Contributor Author

paulgevers commented Nov 24, 2017

@cigamit works better. Can it be that the current code doesn't accept a Data Input Method again after it got whitelisted (after first being invalidated)? The current code does successfully block, but doesn't seem to unblock after I accept it. I currently have to first add the command, update the whitelist manually or with the CLI command and then save the Data Input Method again it seems.

The Data Input Method page ¹ always says "White List Verification Succeeded." whether or not the command is accepted or rejected. That is confusing and looking at the code also not intended.

¹ http://localhost/cacti/data_input.php?action=edit&id=4

Also, cacti.log doesn't mention anything at low logging level, except a reduced amount of RRD's processed. Did I miss logging (at higher logging level) or is this something that should/could be improved at level "low" already?
<< never mind this remark, I now find

2017/11/24 10:41:50 - CMDPHP ERROR: Whitelist entry failed validation for Data Input: Unix - Get Load Average[ 4 ].  Data Collection will not run.  Run CLI command input_whitelist.php --audit and --update to remediate.
2017/11/24 10:41:50 - CMDPHP WARNING: Data Input 4 failing validation check.```

@paulgevers
Copy link
Contributor Author

By the way, I am seeing the following errors in my cacti.log. I don't think they are related to my backport or the changes under discussion, but I'd like your opinion.

2017/11/24 10:38:26 - CMDPHP PHP ERROR NOTICE Backtrace: (/data_input.php: 64 data_edit)(/data_input.php: 473 draw_edit_form)(/lib/html_form.php: 53 CactiErrorHandler)(/lib/functions.php: 4408 cacti_debug_backtrace)
2017/11/24 10:38:26 - ERROR PHP NOTICE: Undefined index: method in file: /usr/share/cacti/site/lib/html_form.php  on line: 57
2017/11/24 10:38:26 - CMDPHP PHP ERROR NOTICE Backtrace: (/data_input.php: 64 data_edit)(/data_input.php: 473 draw_edit_form)(/lib/html_form.php: 57 CactiErrorHandler)(/lib/functions.php: 4408 cacti_debug_backtrace)
2017/11/24 10:38:26 - ERROR PHP NOTICE: Undefined index: method in file: /usr/share/cacti/site/lib/html_form.php  on line: 61
2017/11/24 10:38:26 - CMDPHP PHP ERROR NOTICE Backtrace: (/data_input.php: 64 data_edit)(/data_input.php: 473 draw_edit_form)(/lib/html_form.php: 61 CactiErrorHandler)(/lib/functions.php: 4408 cacti_debug_backtrace)
2017/11/24 10:38:26 - ERROR PHP NOTICE: Undefined index: friendly_name in file: /usr/share/cacti/site/lib/html_form.php  on line: 81
2017/11/24 10:38:26 - CMDPHP PHP ERROR NOTICE Backtrace: (/data_input.php: 64 data_edit)(/data_input.php: 473 draw_edit_form)(/lib/html_form.php: 81 CactiErrorHandler)(/lib/functions.php: 4408 cacti_debug_backtrace)
2017/11/24 10:38:26 - ERROR PHP NOTICE: Undefined index: method in file: /usr/share/cacti/site/lib/html_form.php  on line: 129
2017/11/24 10:38:26 - CMDPHP PHP ERROR NOTICE Backtrace: (/data_input.php: 64 data_edit)(/data_input.php: 473 draw_edit_form)(/lib/html_form.php: 110 draw_edit_control)(/lib/html_form.php: 129 CactiErrorHandler)(/lib/functions.php: 4408 cacti_debug_backtrace)

cigamit added a commit that referenced this issue Nov 25, 2017
@cigamit
Copy link
Member

cigamit commented Nov 26, 2017

Paul, update your global_form.php. Everything should work at this point.

@paulgevers
Copy link
Contributor Author

@cigamit the errors are indeed gone. My other remarks still hold though:

@cigamit works better. Can it be that the current code doesn't accept a Data Input Method again after it got whitelisted (after first being invalidated)? The current code does successfully block, but doesn't seem to unblock after I accept it. I currently have to first add the command, update the whitelist manually or with the CLI command and then save the Data Input Method again it seems.

The Data Input Method page¹ always says "White List Verification Succeeded." whether or not the command is accepted or rejected. That is confusing and looking at the code also not intended.

¹ http://localhost/cacti/data_input.php?action=edit&id=4

Is it possible that I missed a commit? I now have 7 commits back-ported:
365730d, 59e8199, 5f052cc, af4f440, 6732e2d, cd8618f and 8263164

@cigamit
Copy link
Member

cigamit commented Dec 2, 2017

Paul, relative to, i tried to reduce the logging by only logging once per data input method on save/create/repopulate. Double checking the other stuff now.

cigamit added a commit that referenced this issue Dec 2, 2017
This commit adds a new function to validate input string from the hash
value.  Should resolve the last outstanding issue.
@cigamit
Copy link
Member

cigamit commented Dec 2, 2017

Okay Paul. Thank goodness for the weekends. I definitely moved to fast on this one. Everything is working as expected now.

@paulgevers
Copy link
Contributor Author

paulgevers commented Dec 3, 2017

The page now correctly shows if it validates, true. But after CLI --update I still need to save once before the input method enables again (see the amount of RRDsProcessed). I ran CLI before the 10:55 run (by the way, the command I whitelist below is "touch /tmp/CVE-2009-4112", so that may be why the "Invalid Responses" are there).

2017/12/03 11:00:03 - SYSTEM STATS: Time:1.6631 Method:cmd.php  Processes:1 Threads:N/A Hosts:1 HostsPerProcess:1 DataSources:15  RRDsProcessed:10
2017/12/03 11:00:03 - POLLER: Poller[1] WARNING: Invalid Response(s), Errors[1]  Device[testavoira] Graphs[testavoira - Load Average, testavoira - Load Average] DS[3]
2017/12/03 10:55:03 - SYSTEM STATS: Time:1.4316 Method:cmd.php  Processes:1 Threads:N/A Hosts:1 HostsPerProcess:1 DataSources:14  RRDsProcessed:9
2017/12/03 10:52:33 - CMDPHP WARNING: Data Input 4 failing validation check.
2017/12/03 10:52:33 - CMDPHP ERROR: Whitelist entry failed  validation for Data Input: Unix - Get Load Average[ 4 ].  Data  Collection will not run.  Run CLI command input_whitelist.php --audit  and --update to remediate.

@cigamit
Copy link
Member

cigamit commented Dec 3, 2017

Are you thinking we need another flag to push out changes if one had previously failed validation and then we correct the validation. I think that this should be an exception. Maybe '--push-updates' option or something. I would not want to push out an entire system as this can be hundreds of thousands of entries in many systems. Let me know what you think.

@paulgevers
Copy link
Contributor Author

I don't fully understand what you mean here, but if you think the current behavior is the right one, than we may try to improve the documentation. Maybe the string about validation failure could mention that one has to save the input method again after fixing the issue by running the CLI. Then at least I would know what to expect. Or a third "verification successful, but not used yet" string.

I think I wouldn't mind the extra option either.

cigamit added a commit that referenced this issue Dec 3, 2017
…1072

This was related to discussion about updating data input method poller
items in a single pass when validation the white list required updating.
Also, change all echo's to print statements.
@cigamit
Copy link
Member

cigamit commented Dec 3, 2017

Paul, I added a cli option called --push to update the poller items when a validation fails and the --update option is being executed. This only affects existing entries. So, it should never have to be used on a new system as it should not have graphs. Another enhancement might be to add this to the installation as we do attempt to discover the localhost device and add graphs. Something else to think about I guess.

@paulgevers
Copy link
Contributor Author

@cigamit sorry that it took so long to get back to you. I should have tested earlier, but I expected it to be all set now.

I just tested with patch 11b559b added to the patch series that I already had. I now indeed had the option to push while updating. However, although the audit now showed the command was accepted, and also http://localhost/cacti/data_input.php?action=edit&id=4 showed the command as accepted, it was still not executed. I still had to save one more time to get the command to run.

@cigamit cigamit added the enhancement General tag for an enhancement label Jan 20, 2018
@cigamit
Copy link
Member

cigamit commented Feb 10, 2018

Being that we have fixed this in the 1.2 branch. I'm closing this.

@cigamit cigamit closed this as completed Feb 10, 2018
@paulgevers
Copy link
Contributor Author

@cigamit did you fix the issue I mentioned in my comment from Dec 22?

@cigamit
Copy link
Member

cigamit commented Feb 10, 2018

Ope, thanks for reminding me. Forgot all about it. Just doing some house cleaning. Running out of bugs. So, now we can get back on the 1.2 train.

@cigamit cigamit reopened this Feb 10, 2018
@netniV
Copy link
Member

netniV commented Apr 25, 2018

Do I need to do anything in the new installer with regards to the whitelisting? Even if it's to display a warning about how to add new methods on a whitelist-enabled system?

@cigamit
Copy link
Member

cigamit commented Apr 26, 2018

Yes, there is a CLI script in the cli directory that will create the whitelist file. You also have to point to in in config.php.

@netniV
Copy link
Member

netniV commented Apr 26, 2018

Do we want to give people that option in the installer?

@cigamit
Copy link
Member

cigamit commented May 11, 2018

Marking this resolved. Testing in my local environment.

@cigamit cigamit closed this as completed May 11, 2018
@paulgevers
Copy link
Contributor Author

Reminder: @cigamit did you fix the issue I mentioned in my comment from Dec 22?

@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
bug Undesired behaviour enhancement General tag for an enhancement
Projects
None yet
Development

No branches or pull requests

4 participants