Security checks on uploads


(Sam Houston) #1

Are there any security checks on uploads from users? IE does Discourse do anything to stop people from uploading corrupt images, binaries, .js scripts, etc?


(Jeff Atwood) #2

You can only upload allowed file types by extension, that is it. So if you have whitelisted .xls, people can upload .xls files.


(Sam Houston) #3

Sure, though I’m wondering if there is anything to protect from corrupted images. Is there any file inspection to make sure that the file is actually an image and not a javascript/executable file?

Right now we just allow the standard, default extensions (jpg, jpeg, gif, etc).


(Jeff Atwood) #4

You can test that on try.discourse.org if you like, e.g. upload corrupted stuff and see what happens.


(Sam Houston) #5

Okay. But can you answer the question?

Not trying to troll you, just trying to get this question answered as we’re in the process of deciding if we should disable file uploads. I’d rather not since it would limit our users from expressing themselves, so I’m wondering if Discourse does any automatic file checking.


(Jeff Atwood) #6

I don’t see what harm it would cause. If an uploaded image is corrupt, for example, it just won’t display as a valid image.


(Sam Houston) #7

We’d have to chain it together with something else, but we were able to upload a javascript file that was renamed to .jpg:
http://try.discourse.org/t/ldtest-for-bad-bad-bad/399

If you want to read more about Unrestricted File Uploads and how they’re exploited, here’s more info: Unrestricted File Upload - OWASP

That page has recommendations for how to make things more secure, go to the Prevention Methods section at the bottom of the page.


(Jeff Atwood) #8

I’m still unclear why this is a problem, the actual markup produced of uploading a JS file as an image is

<img src="/uploads/default/398/a23952be96fdb105.jpg" width="null" height="null">

It’s conceptually no different than uploading a bad image file. And when retrieving the file we return

Content-Length:0
Content-Type:image/jpeg

You say

And yet when I look there

And some special recommendations for the developers and webmasters:

  • Never accept a filename and its extension directly without having a white-list filter.

That is the first thing I mentioned above, and we have done this for ages. Only extensions on the whitelist are accepted.

Can you provide more details on what the issue is, exactly? Maybe with a repro on try?


(Kane York) #9

I agree, this is a non-issue. Windows runs files by their extension; extensions are whitelisted. On Linux, a downloaded file will never be marked executable by the browser. Sure, you could upload an ELF named as .jpg, but good luck getting people to chmod +x and run it.


(Mittineague) #10

Just testing

…/test.svg

EDIT
Doesn’t seem to be a problem unless the link is clicked

File has been deleted


(Jeff Atwood) #11

That’s valid, it has been known for a while that SVG can run JavaScript. It’s part of the spec, etc.

.svg is also specifically whitelisted here, and that’s not a default, the default whitelist for file upload is .jpg .jpeg .png .gif


(Khoa Nguyen) #12
Connection:Keep-Alive
Content-Encoding:gzip
Content-Length:261
Content-Type:text/html

Browser are treating it as HTML file


(Sam Houston) #13

Thanks, Jeff. I’ll work on getting more details and perhaps see if we can do a repro for you. This is a bit out of my depth, so I’m chatting with others to see what can be done to exploit this particular issue.


(Sam Houston) #14

Hi @codinghorror , our team hasn’t had the time to try to deep dive into things and exploit Discourse on this issue. Though I did see some recent news from Ebay that describes exactly what I was asking about:
https://threatpost.com/ebay-fixes-file-upload-and-patch-disclosure-bugs/111898

Does Discourse do any header checking on images that are uploaded?


(Jeff Atwood) #15

Let’s see if we can exploit it first, following those instructions.

  1. Rename exe to gif
  2. Upload
  3. Get people to click on it and run it

I am on a Windows system so this won’t be much for people who aren’t…

  1. Renamed calc.exe to calc.gif. File size 912 KB (933,888 bytes)

  2. Upload it

  3. Provide link to the file (I guess adding the .exe back on at the end… somehow?) Discourse Meta

In theory this would work for vulnerable PDFs as well, but PDF is a valid upload type, and vulnerable PDF viewers are the problem there, like if the browser’s GIF handling had some kind of buffer overflow.

Ok so I

  1. Download the file. Does seem about the right file size, 911 KB (933,632 bytes)

  2. Rename it to .exe

  3. Run it and get

    Windows SmartScreen has prevented an unrecognized app from running. Running this app might put your PC at risk.

  4. Ok so select file properties, Unblock

  5. Run it and get


(Kane York) #16

ImageMagick editing the images strikes again!


(Sam Houston) #17

@codinghorror I’m not sure what that proves. I was able to upload an HTML file that I renamed to .gif, then download the file locally, rename the file to .html and run the HTML in my browser.


(cpradio) #18

That if you can get someone to go through that amount of work, they deserve to receive the end result. :smile:


(Jeff Atwood) #19

Well the article you cited says

eBay has fixed a pair of security vulnerabilities in its site that could enable attackers to upload executable files disguised as benign file types, construct full path URLs and then point victims to them through drive-by download attacks.

That clearly would not work here, since I can’t repro it. Can you?

As for HTML pages being uploaded, I still don’t understand what the “exploit”. If you made a HTML page that says

Discourse causes cancer!

And wanted us to serve it, Ok that would be bad. We don’t want discourse.org to serve a web page that tells people Discourse causes cancer.

But that doesn’t work either. You could rename that file and upload it as a gif but it will always be served as a GIF! Never as a HTML file.

At the point where you download the GIF, rename the file on your local computer and click on it… that’s not an exploit.

I agree that it’s not a bad idea to run some additional checks on images to make sure they are images, but there’s no exploit around it here that I can reproduce.


(Sam Houston) #20

If I can upload an HTML page I can do what’s called a Cross Site Scripting (XSS) attack, as some browsers will render gifs as HTML. Neal Poole from Facebook wrote about a Wordpress vuln like this back in 2011

The HTML file could have javascript in it, which I could use to do bad things.

Again, I’m not a security researcher (though that’s what our community is made up of), so I can’t speak at length about XSS or test this thoroughly. I’m just wondering if you guys do anything to check headers of uploads. If you could answer that question for me, that would be awesome.