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

Security Vulnerability: ActionDispatch::Static serves hidden directories and files #5987

Closed
acatighera opened this issue Apr 26, 2012 · 10 comments

Comments

@acatighera
Copy link
Contributor

Files in the "/public" directory are served even if they are hidden or within a hidden dir. This is a serious security issue. Here is an example, people using SVN will have their ".svn/entries" file exposed. This file contains the location and usernames for their SVN repo. An attacker could use this info to commence a brute force attack to gain access to the repo!

For some reason github won't let me fork rails so I created a patch. Here it is: https://raw.github.com/gist/2495435/65883bcecb76916529db2a761c120adcd68ede54/static_file_fix.patch

@Aupajo
Copy link
Contributor

Aupajo commented Apr 26, 2012

I'm not sure that, just because a directory starts with a dot, there's an implicit promise to hide those directories in the context of a web server.

This feels like something you'd handle in your production server configuration. I would presume many (if not most) Rails apps in production don't use Rails itself to serve their public directory (they might use, say, nginx), so I think there's a bit of a danger in adding this patch. If directories starting with a dot are hidden in Rails by default, the developer might make the assumption that those directories will also be hidden by whichever web server they do end up using to serve their public folder. I'd side with sticking with what we have.

Specifically for .svn folders, again I would have thought most people would have this factored into their deploy process.

@acatighera
Copy link
Contributor Author

Sure dot does not strictly mean hidden but in 99.999% of cases it does mean hidden. I don't see the harm in hiding these files.

Even if you are using nginx to serve static files this is still an issue. If the public directory exists then a potential attacker will still be able to hit ".svn/entries", at least with the recommended nginx config in rails official guide.

I'm pretty sure most people will overlook this, it is somewhat obscure.

@sikachu
Copy link
Member

sikachu commented Apr 26, 2012

This is also not a right place to report security issue ... If it is really a critical issue.

@acatighera
Copy link
Contributor Author

I wouldn't say it's a critical issue since this issue is already known and all the files in the public dir are well ... public.

"the developer might make the assumption that those directories will also be hidden by whichever web server": Most developers probably are unaware of this issue and would not even know to make that assumption. Developers who would be aware of those files would already understand how those files are served and make sure that they aren't public.

@carlosantoniodasilva
Copy link
Member

Well, although it's possible to, technically ActionDispatch::Static is not supposed to be used in production, the web server should take care of handling static stuff. As you already mentioned, even nginx would serve such files, so I don't see how changing ActionDispatch::Static would help.

So, I have to agree with @Aupajo that the developers are responsible about what they're serving as static files, and they should remove anything that can be harmful during the deployment process.

@acatighera
Copy link
Contributor Author

I don't see any harm in adding it.

"ActionDispatch::Static is not supposed to be used in production." I don't see this claim anywhere in the docs. Is it explicitly stated anywhere? Furthermore, what if someone is not using a downstream web server like nginx? Not a common setup but I could think of a few scenarios where someone might do this. In this case, there would be no way to prevent these files from being served without hacking rails or capistrano.

Also, the setup in the rails guides on asset pipeline only has a directive for ~/assets. For files at the root level of the public folder changing ActionDispatch::Static would help. This would be the most common location to be scanned by an attacker.

@carlosantoniodasilva
Copy link
Member

@acatighera this goes to your production.rb file, in config/environments:

 # Disable Rails's static asset server (Apache or nginx will already do this).
 config.serve_static_assets = false

If you run rake middleware under development mode, you'll see that ActionDispatch::Static is at the top of the list.. if you run rake middleware RAILS_ENV=production, it won't be there anymore, due to that line.

The guides or docs could be more explicit about this, yeah I agree. A possible harm would be some user that expects or rely on this to work in other scenarios that we may not be considering here. I'm unsure that static not serving . files from public is the right way to go, more docs would serve the purpose I believe.

@acatighera
Copy link
Contributor Author

I guess we will have to agree to disagree because like I said, what if they aren't using Apache or nginx? There is no way to fix without monkey patching rails or having a hack in capistrano if there is no downstream webserver. That's sort of the heart of the problem, apache and nginx let you configure it somehow, however there is no way to configure it at rack/rails/thin layer. At least not a way that I know of.

@jeremy
Copy link
Member

jeremy commented Apr 26, 2012

@acatighera, please report security issues to security@rubyonrails.org

@carlosantoniodasilva
Copy link
Member

I think that if they are not using apache or nginx, they'll likely be using another web server, that would allow to configure something like this if that's a real problem:

# nginx - taken from http://www.unixpearls.com/nginx-how-to-deny-dot-file-requests/
location ~ /\. { deny all; access_log off; log_not_found off; }

And in case someone is not using anything that lets it to be configurable, it's always possible to wrap a new middleware that handles that, and swap ActionDispatch::Static.

I'm going to close this issue for now, please follow @jeremy's instructions to report as a security issue. Thanks.

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

No branches or pull requests

5 participants