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

Unrestricted File Upload (Remote Code Execution) Vulnerability #899

Closed
tresacton opened this Issue Jul 21, 2015 · 39 comments

Comments

Projects
None yet
@tresacton

tresacton commented Jul 21, 2015

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.

@rwarasaurus

This comment has been minimized.

Member

rwarasaurus commented Jul 21, 2015

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

@TheBrenny

This comment has been minimized.

Member

TheBrenny commented Jul 21, 2015

@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

This comment has been minimized.

tresacton commented Nov 18, 2015

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

This comment has been minimized.

Member

daviddarnes commented Nov 18, 2015

@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

This comment has been minimized.

joshvickerson commented Nov 18, 2015

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

This comment has been minimized.

Member

daviddarnes commented Nov 18, 2015

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

@TheBrenny

This comment has been minimized.

Member

TheBrenny commented Nov 18, 2015

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

This comment has been minimized.

tresacton commented Nov 23, 2015

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

This comment has been minimized.

tresacton commented Nov 23, 2015

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

This comment has been minimized.

ghost commented Feb 22, 2016

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

This comment has been minimized.

Member

TheBrenny commented Feb 23, 2016

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
#899 (comment)
.

@knoy

This comment has been minimized.

knoy commented Feb 23, 2016

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

This comment has been minimized.

tresacton commented Feb 23, 2016

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.

@Shad0wSt3p

This comment has been minimized.

Shad0wSt3p commented Feb 23, 2016

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

This comment has been minimized.

kckrinke commented Feb 23, 2016

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

@bp256r1

This comment has been minimized.

bp256r1 commented Feb 23, 2016

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

This comment has been minimized.

mikepmtl commented Feb 23, 2016

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?

@bp256r1

This comment has been minimized.

bp256r1 commented Feb 23, 2016

@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

This comment has been minimized.

ghost commented Feb 23, 2016

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

This comment has been minimized.

Member

TheBrenny commented Feb 23, 2016

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
#899 (comment)
.

@bp256r1

This comment has been minimized.

bp256r1 commented Feb 23, 2016

@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

This comment has been minimized.

mikepmtl commented Feb 23, 2016

@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.

@bp256r1

This comment has been minimized.

bp256r1 commented Feb 23, 2016

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

This comment has been minimized.

tresacton commented Feb 23, 2016

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

This comment has been minimized.

mikepmtl commented Feb 23, 2016

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

This comment has been minimized.

aoloe commented Feb 23, 2016

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

This comment has been minimized.

tresacton commented Feb 23, 2016

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

This comment has been minimized.

aoloe commented Feb 23, 2016

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

This comment has been minimized.

Member

daviddarnes commented Feb 23, 2016

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

This comment has been minimized.

tresacton commented Feb 23, 2016

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

@Shad0wSt3p

This comment has been minimized.

Shad0wSt3p commented Feb 23, 2016

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.

@Shad0wSt3p

This comment has been minimized.

Shad0wSt3p commented Feb 23, 2016

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

This comment has been minimized.

Member

daviddarnes commented Feb 23, 2016

@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.

@tresacton

This comment has been minimized.

tresacton commented Feb 23, 2016

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

This comment has been minimized.

Member

daviddarnes commented Feb 23, 2016

@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.

@rwarasaurus

This comment has been minimized.

Member

rwarasaurus commented Feb 23, 2016

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

This comment has been minimized.

Member

daviddarnes commented Feb 23, 2016

I guess blind rage does work…

@matt-

This comment has been minimized.

matt- commented Feb 23, 2016

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

This comment has been minimized.

ghost commented Feb 23, 2016

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.

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