anchorcms / anchor-cms

A lightweight blog CMS for PHP
http://anchorcms.com/
GNU General Public License v3.0
3.32k stars 575 forks source link

Unrestricted File Upload (Remote Code Execution) Vulnerability #899

Closed tresacton closed 8 years ago

tresacton commented 9 years ago

I identified a vulnerability in the AnchorCMS product. I was testing 0.9.2 as this is the version is the latest release published on the AnchorCMS website, however, it is possible this issue is present in other versions as well.

Vulnerability: Unrestricted File Upload (Remote Code Execution) Preconditions: Authentication as user of any role (i.e.: ‘Admin’, ‘Editor’, or ‘User’) File Upload needs to be enabled for posts Attack Scenario: • Attacker brute forces a valid username and password combination (or the attacker is a legitimate user with any role) • Attacker enables the file upload facility, if it isn’t already enabled • Attacker uploads a PHP file designed to execute shell code • All PHP files are automatically executed when the file is navigated to in the browser • The attacker gains access to the underlying server and can now execute arbitrary code in the context of the web server user account (in my case, this was the 'www' apache user account)

Note: David (from AnchorCMS) advised me to publicly disclose this by creating a GitHub issue against this repository.

saltandvinegarcrisps commented 9 years ago

If the attacker can't brute force the user/pass would this still be an issue?

TheBrenny commented 9 years ago

@rwarasaurus I'd believe so, this is related to issue #873.

I would've made something, but I don't know what each role's permissions will be. I spoke about it with @CraigChilds94 briefly, but not enough to gather the full details.

tresacton commented 8 years ago

Hi all,

Given that I was asked to disclose this in a public forum, and several months have passed, I'm going update the CVE entry (identifier CVE-2015-4698) with the relevant details, such as the nature of the vulnerability.

Cheers,

T.A.

daviddarnes commented 8 years ago

@tresacton it seems that this issue is not really an issue. The first two steps to reproduce this vulnerability is to brute force a username and password, but isn't that true with any system?

I don't think this requires a report.

joshvickerson commented 8 years ago

Well, it doesn't require brute force, necessarily. It just requires a user in any role to be logged in, meaning that a user with malicious intent could execute an attack. Seems like as good a reason as any to restrict file upload types.

daviddarnes commented 8 years ago

@joshvickerson I'm not saying we should take action, I just think we shouldn't be marking it as such a drastic vulnerability.

TheBrenny commented 8 years ago

While the two of you both have valid statements, I'd have to agree with David, here. Its not sooooo drastic, seeing as admins would only create users for the purpose of adding a comment, or creating a forum. The latter of which is not the original intent for Anchor. The former? Why not just throw in Disqus? Its free and it works, and according to the many results on Google, it works almost seamlessly. (The almost comes from needing to play around with your chosen theme to get things up and running)

tresacton commented 8 years ago

It is a serious vulnerability regardless. Just because a user has access to the web application interface, does not mean that they should have access to the underlying operating system, which is what happens when this vulnerability is exploited. In some cases, this may be also be the root account.

Furthermore, bruteforcing is not necessarily required for exploitation, other attack vectors or vulnerabilities can be used to leverage or facilitate exploitation of this issue.

tresacton commented 8 years ago

Allow me to illustrate the severity with another attack scenario example.

Let's say I stored sensitive intellectual property, or usernames and passwords, or credit card information for customers on a server - or even, that this server was on the same network as some other machines I own that store this data.

On the same server, I install Anchor CMS. I provide my 'content manager' with access to the AnchorCMS interface to add new HTML pages to my site. They exploit this vulnerability and now have access to the underlying operating system. With this access, they retrieve everything else that's stored on this server and/or use this server as a foothold/pivot point to further exploit the other systems and services on my network.

This is just an example. There are very many other ways that Remote Code Execution vulnerabilities can have a devastating impact. Remote Code Execution vulnerabilities are arguably one of the most severe types of vulnerabilities due to the potential consequences of compromise.

I hope this helps.

ghost commented 8 years ago

All PHP files are automatically executed when the file is navigated to in the browser

Can this be fixed for user-uploaded content then?

TheBrenny commented 8 years ago

Anchor is a platform that is generally used for soloists. If you have a content manager, then you should be using something a bit more hardcore, like WP. On 22 Feb 2016 23:09, "kehugter" notifications@github.com wrote:

All PHP files are automatically executed when the file is navigated to in the browser

Can this be fixed for user-uploaded content then?

— Reply to this email directly or view it on GitHub https://github.com/anchorcms/anchor-cms/issues/899#issuecomment-187177551 .

knoy commented 8 years ago

Its not sooooo drastic, seeing as admins would only create users for the purpose of adding a comment, or creating a forum. The latter of which is not the original intent for Anchor. The former? Why not just throw in Disqus? Its free and it works, and according to the many results on Google, it works almost seamlessly.

Anchor is a platform that is generally used for soloists. If you have a content manager, then you should be using something a bit more hardcore, like WP.

Quit trying to minimize the issue. Trying to say that people should really be using Wordpress or Disqus or whatever is a shitty way of avoiding the problem. This is a serious vulnerability and one that should be extremely easy to fix.

Just patch the vuln.

tresacton commented 8 years ago

Anchor is a platform that is generally used for soloists. If you have a content manager, then you should be using something a bit more hardcore, like WP.

This doesn't make the issue any less severe and is rather irrelevant. Despite it being irrelevant, I would ask why there is a notion of 'roles' if this platform is for 'solosits'?

Surely, given the length of this thread, it would have been faster to just implement a patch instead of attempting to justify that "RCE is not a serious vulnerability"?

I'm confident that your goals as developers are to create a product that follows good coding practices, and can be considered mature. Arguing about whether or not this is a serious issue (which it clearly is), is not going to help you achieve that.

If the reason this hasn't been patched boils down to not knowing how code a patch to mitigate this high risk vulnerability, please note that OWASP is an excellent resource for developers - they provide many PHP examples for mitigating common and severe web application security issues. Alternatively, I'm sure that a quick google search would yield many results with tutorials demonstrating best practices for file and user input handling.

0x90n commented 8 years ago

If the attacker can't brute force the user/pass would this still be an issue?

This is why most companies perform internal testing - from an authorized account...they assume brute-force or account compromise has already taken place. By saying that this does not require a report at all is very foolish; and I think it being publically disclosed already means you may have put your own users at risk.

I feel like the effort involved to patch this does not even come close to the reputational loss suffered when a client/user gets stung by a public, unpatched CVE.

kckrinke commented 8 years ago

I feel like the effort involved to patch this does not even come close to the reputational loss suffered when a client/user gets stung by a public, unpatched CVE.

Just echoing the above. Very well said @Shad0wSt3p

ghost commented 8 years ago

Hello there,

Asking your clients, and users to migrate away from AnchorCMS as a remediation for a remote code execution (RCE) vulnerability which you introduced into the code base does not seem like an adequate solution to me.

Then again, I am not too familiar with your code base, and perhaps it would be infeasible for you to perform proper input sanitisation? If you need assistance with this, some of your clients may be able to help.

In any event, advising your clients, and users to migrate away from your product because you're unsure of how to fix a bug seems a little silly to me.

The key take away of the vulnerability disclosure by @tresacton is that any user which can log-in to AnchorCMS can execute arbitrary code on the system which is hosting AnchorCMS. This does not seem like it's expected, or even well-defined behaviour to me.

Perhaps we should follow the fix suggested by @TheBrenny, and migrate to another platform such as Wordpress, or Drupal? That's the official bug fix, right?

mikepmtl commented 8 years ago

I am only looking at Anchor, have not installed it. But would it not be easy to simply "whitelist" what files types may be uploaded?

ghost commented 8 years ago

@mikepmtl That wouldn't be an appropriate solution in my opinion because then you wouldn't be able to host PHP files using AnchorCMS. I am unaffiliated with this project, but I might be able to help you all determine the appropriate remediation steps.

As a remediation, you may have to set up a blacklist of potentially dangerous functions, or use regular expressions in order to determine if something looks bad (kind of like how you detect SQLi and stored XSS), and then simply prevent users from uploading PHP files which are flagged as (potentially) malicious based on a set of criteria which you devise.

This is an important issue though, it shouldn't be possible to own your friend Dave's box just because he's letting you look at what he's hosting on his CMS.

ghost commented 8 years ago

blacklist of potentially dangerous functions, or use regular expressions in order to determine if something looks bad

Sounds extremely weak (base64_encode and the protection is gone). Why not chmoding user uploads to 660?

TheBrenny commented 8 years ago

All above,

I'm not saying that this isn't an issue; of course it is, and RCE can be deadly to the server.

However, what I am saying, is that unless an attacker has managed to get themselves into the admin panel, this is a (albeit slightly) less serious issue.

No I am not a professional in security, and yes, all of you probably have a lot more knowledge about security than me, so I understand that this issue is still pretty serious.

When I was mentioned to migrate away, it was a reference to the comment made by having a "Content Manager". If you're group/company/team is so large that you are not uploading/creating content for yourself, THEN you should use a more advanced CMS, let alone your CM might've changed to a different CMS in the first place.

For my own knowledge, I don't understand how a file upload can result in an RCE vulnerability. This is also why I'm thinking that this isn't as serious as it possibly could be.

Sorry for creating a heated discussion, due to my lack of understanding in this subject area. I'm sure a fix will be created soon, but there aren't many contributors at the moment. On 23 Feb 2016 10:48, "Tyler" notifications@github.com wrote:

Hello there,

Asking your clients, and users to migrate away from AnchorCMS as a remediation for a remote code execution (RCE) vulnerability which you introduced into the code base does not seem like an adequate solution to me.

Then again, I am not too familiar with your code base, and perhaps it would be infeasible to perform proper input sanitisation?

In any event, advising your clients, and users to migrate away from your product because you're unsure of how to fix a bug seems a little silly to me. Perhaps we should follow your advice, and migrate to another platform such as Wordpress, or Drupal?

— Reply to this email directly or view it on GitHub https://github.com/anchorcms/anchor-cms/issues/899#issuecomment-187463133 .

ghost commented 8 years ago

@kehugter: why does this sound weak to you? Sure, if you base64 encode that function call the protection is gone, but, it's better than nothing. Even then, it's only a recommended security control.

You could implement the control I proposed, and more (e.g. you could also ensure that all PHP files are executed by a user in a chroot jail).

The goal of my suggestion was more so geared towards preventing malicious files from being uploaded in the first place, however, steps clearly need to be in place in order to reduce the attack surface that an attacker has access to.

mikepmtl commented 8 years ago

@kehugter even with permissions of 400 PHP will still interpret it.

But there is no reason to need to keep PHP as a .php, .js, .py or whatever file. It can be kept as a .txt file and not interpreted at all.

ghost commented 8 years ago

That's totally okay, @TheBrenny. Nobody is an expert on everything. Perhaps @tresacton could demonstrate the technical proof of concept that he has prepared in order to illustrate how CVE-2015-4698 can be practically exploited from a non-administrative account?

That might be the best next logical next step forward, since it will show that largely untrusted users can take control of the system, and do whatever they like. The bottom line is that this vulnerability allows for any logged-in user to do anything that their heart desires on the underlying system.

tresacton commented 8 years ago

For my own knowledge, I don't understand how a file upload can result in an RCE vulnerability. This is also why I'm thinking that this isn't as serious as it possibly could be.

I'm always happy to elaborate and help any developer to reproduce results. Please give this a go:

# exploit.php
<?php
$output = shell_exec('ls -la');
echo "<pre>$output</pre>";
?>
  1. create the above file, call it exploit.php
  2. navigate to 'Extend' and enable a File Upload field for Posts (if not already enabled)
  3. Create a new post, and upload the PHP script you just created (exploit.php)
  4. (optional) log out entirely
  5. open a browser and navigate to the PHP file by its URL in the app
  6. notice that the command in the script (in this case 'ls -la') was executed on the server, and the result of the command has been returned in your browser.
mikepmtl commented 8 years ago

You can easily handle this as well at the web server level.
Simply do not pass any .php files to the PHP interpreter under a specific directory.

Or generally what I do in my software is for a different reason, permissions, but solves this issue as well. All files that have been uploaded and made available for download go through a controller that verifies that you have permissions to download it.

In that case NO uploaded files are even in the web document root of the project so can never be "executed".

aoloe commented 8 years ago

without being a security expert, with php the simplest and most secure solution seems to be to avoid saving uploads under their original name.

that way will also allow multiple uploads with the same name (but possibly a different content: report final.docx anyone?).

of course, you will have to store somewhere the original file name in order to serve it back under that name through a download script. (or simply add a random six letter extension that you strip on download).

a well crafted rewrite rule will allow you to define unique urls that serve the correct file (and if rewrite is deactivated no harm can be done... in the best case, the are unreachable since they should be stored outside of the document root in order to allow some access control)

tresacton commented 8 years ago

I could very well be misunderstanding what you wrote, but I don't understand how this mitigates the issue if the file is still being served?

aoloe commented 8 years ago

if there is no .php at the end (no extension), the files will be served as raw text (if ever) and not executed on the server. the attacker will see the source code of the file she has uploaded (and not the result of the script)

daviddarnes commented 8 years ago

I've been looking through these comments just now (it's morning where I am) and I'm appalled at the tone of voice and lack of respect on this Issue.

The reason I asked that this issue be raised on GitHub was in the hope that the open source community would come up with solutions to the problem in a mature manner. From what I can see most of us are just throwing blame around and pointing fingers.

I'm not asking for you to provide the golden bullet for this problem, especially since I wouldn't know where to start on such an issue, but I am asking if you could work together on simply providing help. Thank you.

Edit: This was a morning explosion of rage and I apologise. The majority of comments here have been helpful.

tresacton commented 8 years ago

Oh ok gotchya. Could be an alright workaround. I recommend exercising caution though as it could be susceptible to null byte attacks if not implemented correctly. Please see the following link for some good approaches to mitigation: https://www.owasp.org/index.php/Unrestricted_File_Upload

0x90n commented 8 years ago

I've been looking through these comments just now (it's morning where I am) and I'm appalled at the tone of voice and lack of respect on this Issue.

Wow wait what? I've seen nothing but helpful discussion on mitigating a key vulnerability here...there has been no tone of voice - the only lack of respect for the issue here was your previous comments saying it does not deserve a report and "its not soooooo drastic."

The reason I asked that this issue be raised on GitHub was in the hope that the open source community would come up with solutions to the problem in a mature manner.

That is exactly what's been happening. And that's out of their own time and effort, helping you. I am really, really confused as to your reply on this. As far as i'm concerned, you should be very thankful to anyone for being concerned about your app, because that's all I can see.

0x90n commented 8 years ago

The security community is very happy to help out anyone without a solid knowledge and understanding of vulnerabilities. No dev is expected to know everything off the bat. However keeping an open mind and taking the humble back seat when something is pointed out is critical towards the cohesion of both communities.

daviddarnes commented 8 years ago

@Shad0wSt3p

Quit trying to minimize the issue. Trying to say that people should really be using Wordpress or Disqus or whatever is a shitty way of avoiding the problem. This is a serious vulnerability and one that should be extremely easy to fix.

Just patch the vuln.

  • @knoy
tresacton commented 8 years ago

Hi David,

Unsure if that was directed at me, but please note that I have taken time out of many of my days in an effort to help improve the security of your product and that of your users.

It is not my intention to point fingers of blame. I have much better things to do with my time and I was also a developer once. I get that you guys are providing an open source product and don't have all the time in the world for this. However it has been sitting here for the better part of 7 months.

If I was a PHP developer I would have submitted a pull request. However, I am not a PHP developer and the best way for me to help is providing guidance. There is nothing wrong with reaching out to a wider open source community for help, but nobody up until that comment, suggested that they needed help. All that I've witnessed from the devs so far was (at least a prolonged and initial) refusal to consider that this is both legitimate as an issue and serious in nature.

If you dont understand something in a bug report or vuln disclosure it would be applaudable to reach out and ask for help or clarification. Unfortunately, ignoring an issue or downplaying its severity sends the wrong message and leaves users sorely unprotected.

Now, noting that Im not a strong php developer (if only this was Ruby :p) is there anything else I can do to help you to get this issue resolved?

daviddarnes commented 8 years ago

@ALL Apologies if my comment seemed harsher than intended. We've had a lot of issues raised in the past on this project and they haven't been met with much respect for the developers. I am not a major contributor to this in fact, I contribute in other areas but I don't work on the core.

@tresacton It's not directed at you. We've had a few issues reported here in the past that have been met with little respect for the original developers. At the same time I'm somewhat powerless to amend this issue as I'm not a PHP developer either, this isn't technically my project. I'm just someone who has commented a lot on this project and contributed in other areas.

@Shad0wSt3p I'm sorry, the tone of my comment was harsher than I intended. We've had some disrespectful people comment on this project and it is sometimes too much to handle. This particular Issue isn't that much of an offender.

saltandvinegarcrisps commented 8 years ago

I've updated the custom field upload input to only accept png, jpg, bmp, gif, pdf file extensions which is better than accepting anything for now, ideally it should check the mime type of the file contents instead of trusting the file extension.

daviddarnes commented 8 years ago

I guess blind rage does work…

matt- commented 8 years ago

I hate to bump a closed ticket, but I thought I would kick in my advice. I have worked in security for the about the last 8 years.

  1. I think limiting the extensions is a good start. You are doing this correctly with a "whitelist". There are a lot of things you don't want. php files, html files (XSS), cgi scripts,....
  2. I would create a new directory for uploads and add instructions to set the file system to not allow execution. something like: chmod 644 uploads/ (This means that the WWW user can read and write (edit) the directory, while everyone else can only read it and no one can execute files from it).
  3. Create a .htaccess for the "uploads" directory that forces the files to download if you browse to them directly (prevents some content sniffing and things like that).

Something like:

<Location /uploads>
    Header set Content-Disposition attachment
</Location>
ghost commented 8 years ago

Bolt has a nice nginx template: https://docs.bolt.cm/installation#nginx-configuring-virtual-host One of the few that includes security as default and not as an extra hardening section.