Update lib/rack/directory.rb #521

Closed
wants to merge 1 commit into from

4 participants

@snyff

File XSS in the path_info

@snyff snyff Update lib/rack/directory.rb
File XSS in the path_info
3e6d46e
@snyff

The same issue is in file.rb line 62

@chneukirchen
Official Rack repositories member

Content-Type is text/plain, how is this XSS?

@snyff
@snyff

No need to do an advisory or get a CVE. I just use Rack and would like this to be fixed since in my use case it could be exploited :)

I submitted another patch to prevent symlinks usage. Not sure if you'd like me to put that as an option to Rack::File or you're happy with the current patch.

Any issue let me know. I can re-submit the same commit as "encoding improvement" if you want to avoid the term XSS ;)

@snyff

If you have a file named something like blah in the directory, the directory listing will be also vulnerable to XSS. I added a patch for this as well

@snyff

I just patch in 2 places (example of basic auth and digest auth) to use secure_compare instead of ==

@rkh
Official Rack repositories member

I still don't understand. HTML escaping in a text document makes no sense. How does the XSS attack work?

@snyff

It can happens with older browsers that will try to load the content even if the content-type is text/plain

@raggi
Official Rack repositories member

This is a browser bug, not a bug in our code. The escaping you propose is invalid for the document type.

@raggi raggi closed this Apr 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment