Okay, so… don’t allow people to upload HTML pages?
The Content-Disposition headers are set on all attachments, and extensions are on a whitelist.
The “semicolon filename” trick doesn’t work, either.
Sorry if I confused you. I’m not talking about uploading .html, I’m talking about uploading .gif file that is actually an html file. As I said in my previous post, I’ve successfully uploaded a gif that was actually html, and some browsers (due to bad mime type handling) will display a .gif as HTML if the gif has html code within it.
Which isn’t good, as it opens you up to XSS and other attacks.
It seems that we’re at an impasse, as you guys refuse to answer my question directly for some reason. Which is disappointing.
If/when our community or our staff finds any security vulnerabilities in Discourse we will report them to your team@ email address.
Regardless of this disappointing thread conversation, I’m looking forward to launching our Discourse forum and I hope to chat with the Discourse team again in the future. I’ve been (mostly) a fan of what we’ve seen so far and the overall setup and config experience has been great.
You’ve whitelisted HTML on your site?
Here all I see is
.jpg, .jpeg, .png, .gif, .svg, .txt, .ico, .yml, .csv, mp3
If you’re referring to renaming an HTML (text) file to gif (binary) what you’ve done is uploaded a corrupt gif file, not an html file.
Or am I missing something?
I don’t see the problem here, I’m pretty sure that we have all the standard holes around this covered. That’s why you’re seeing such a lax response - you’ve pointed out that these standard holes exist, but you haven’t been able to show where exactly Discourse is vulnerable or causes a vulnerability.
Sure, you can whitelist the * extension and then people can upload .exe files, but you kinda explicitly agreed to that when you whitelisted all extensions.
I haven’t changed the whitelist from the default, which is just jpg/jpeg/png/gif . Currently, you can rename an HTML file to gif, then upload that gif to Discourse. Discourse will serve that gif to a user, while some browsers will display that gif as HTML due to a Mime-type handling issue and content sniffing issues that some browsers have. Neal at Facebook wrote about this back in 2011 because Wordpress had the same issue.
Discourse may be able to mitigate this issue because you guys don’t allow old versions of IE, but users could still upload Evil Stuff ™ as gifs (or svgs, which are also exploitable) and use someone’s Discourse as a server to host their files they use for XSS (or whatever they want to do).
Once again, I am not talking about an admin allowing anything beyond the default whitelisted file types.
@Riking - My intention with this thread was not to report a vulnerability, as I’m not a security researcher. My intention, as a customer of Discourse, was to ask you guys a direct question (Do you do any header checking, file checking, etc beyond whitelisting on Uploads), and get a direct answer.
I just looked at the code on Github and as far as I can tell (I took 1 year of CS so I could be wrong), Discourse does not do any header checking for files that are uploaded. Discourse just checks to see if the filetype is in the whitelist or not, and then displays that file to the user.
Well, let’s look at the headers of an attachment:
$ curl -I //assets-meta-cdck-prod-meta.s3.dualstack.us-west-1.amazonaws.com/original/3X/8/3/836737a0f45d64a24bc6a176431a9698a65c10b2.txt
HTTP/1.1 200 OK
Date: Tue, 31 Mar 2015 21:06:57 GMT
Server: nginx
Content-Type: text/plain
Last-Modified: Fri, 13 Mar 2015 22:03:25 GMT
Content-Disposition: attachment; filename="discourse.conf.txt"
Cache-Control: private
X-UA-Compatible: IE=edge
X-Clacks-Overhead: GNU Terry Pratchett
Strict-Transport-Security: max-age=31415926
Content-Length: 1316
Accept-Ranges: bytes
Via: 1.1 varnish
Age: 0
X-Served-By: cache-sjc3131-SJC
X-Cache: MISS
X-Cache-Hits: 0
X-Timer: S1427836017.432519,VS0,VE63
Hmm, I don’t see the no-sniff directive in there, actually.
You don’t need to worry about IE<9 because Discourse won’t run on those, and the URL bar won’t update for IE=9.
1 Like
Can you name any current browser that is still vulnerable to this?
That was a long time ago. I guess a lot has changed since then. AFAIK none of the issues mentioned there are possible with the default configuration of Discourse.
Yes, X-Content-Type-Options: nosniff is definitely missing (only for files served from fastly?). See also List of useful HTTP headers.
1 Like
I understand the desire, but before I can spend engineering time on this we need a repro. Nobody has been able to produce a repro and the whitelist and proper headers have been in place long before this topic was started.
As far as old browsers go we don’t even work on ie8 and will likely be dropping ie9 by the end of 2015.
I agree more checks can’t hurt but without a repro it is hard to justify spending time on this versus other features that directly benefit our users and customers.
Fair enough, my intention was not to lobby for a feature or request some Dev work. I just wanted to ask the question about what you guys do currently.
Like I said earlier, if/when we find any issues we can reproduce we will let you know.
2 Likes
Well, let’s give it a whirl:
- Following the instructions at http://marcoramilli.blogspot.de/2014/01/hacking-through-image-gif-turn.html, I’ve created a GIF with an embedded JavaScript.
- The created file is available here: http://www.elberet.de/~jens/image.gif_malw.gif.zip
- Uploaded the file to Discourse: https://meta.discourse.org//assets-meta-cdck-prod-meta.s3.dualstack.us-west-1.amazonaws.com/original/3X/f/5/f56e6de2af55bfca363572abefabe57c3764a27e.gif
- Hit F12 and run
document.write("<script src=\"https://url.to/original.gif\" type=\"text/javascript\"></script>");and you get a JavaScript alert. - Now run
document.write("<script src=\"https://meta.discourse.org//assets-meta-cdck-prod-meta.s3.dualstack.us-west-1.amazonaws.com/original/3X/f/5/f56e6de2af55bfca363572abefabe57c3764a27e.gif\" type=\"text/javascript\"></script>");… and it doesn’t work anymore.
Discourse postprocesses all uploaded images (as far as I’ve understood the OptimizedImage code). All files you’re going to download from Discourse will be valid images, even if the image data they contain is visual garbage.
I would be far more concerned about legal repercussions for hosting criminal material than serving malware…
(Oh, and it still works as a GIF:
… at least sort of. I probably shouldn’t have used an animated GIF, the detected dimensions are out of whack so it gets lightboxed.)
4 Likes
Bumping this topic as we do have a report of this for SWF files, at least:
http://labs.detectify.com/post/86302927946/the-lesser-known-pitfalls-of-allowing-file-uploads
- An attacker creates a malicious Flash (SWF) file
- The attacker changes the file extension to JPG
- The attacker uploads the file to
victim.com - The attacker embeds the file on
attacker.comusing an tag with typeapplication/x-shockwave-flash - The victim visits
attacker.com, loads the file as embedded with the<object>tag - The attacker can now send and receive arbitrary requests to
victim.comusing the victims session - The attacker sends a request to
victim.comand extracts the CSRF token from the response
This, at least is a repro… I can support adding some structure to our ImageMagick code paths which are already attempting to process the uploaded images in various ways (thumbnail, get dimensions, etc) – it seems to me we should be able to detect errors in that code path and reject the “image” file?
1 Like
We were already checking the dimensions of the uploaded images but the code wasn’t working. Improved it and added a test to make sure it doesn’t regress
https://github.com/discourse/discourse/commit/a9099f9e2383affcdca60e86ab167216f1ec68b2
3 Likes
OK I found a sample SWF file and changed the filename to end with .jpg then attempted to upload it on try.discourse.org:
So far so good! I got
Sorry, but we couldn’t determine the size of the image. Maybe your image is corrupted?
Looks solid to me, any other chance of regressions here @zogstrip?
This “use case” is now fully tested. So unless they find another way of uploading an image, we’re pretty safe for that use case.
3 Likes
