Reevaluating file extension handling: Allow all extensions

[corrideat]

Problem

In PR #1877, there's a list of 'common' extensions defined in CHAT_ATTACHMENT_SUPPORTED_EXTENSIONS. This list is used to decide which files to allow, and files without any of those extensions will fail with a message that reads 'Unsupported file type'.

I'm not sure what the purpose of such a list was. I could understand if it's being used to decide how to process a file (in this case, I'd only use it as a hint rather than as an absolute), but in this case it's only used for the purpose of allowing or blocking the files that can be submitted.

Furthermore, the list includes some files that are not common at all outside of (web)dev circles, like php, sh, js, vue or xul while excluding others that are common, like flac, dwf (AutoCAD), key (Keynote), etc. The point is that such lists can't possibly be comprehensive enough without being too long to the point that they lose meaning.

If the purpose was to prevent 'dangerous' files, note that some of those extensions can also be malicious. For example, js files in Windows can do anything an executable can do and are a common way of distributing malware (the same applies for js files in general on all platforms, although someone using, e.g., node, presumably knows what they're doing). Similarly, pdf files can contain scripts and can be malicious. The same goes for jar files (can contain arbitrary Java code), doc (can contain macros), etc. So, the list also fails at protecting users from potentially dangerous file types.

Solution

I think the list should be removed entirely, or at least not used to decide the file types that are allowed. Some alternatives are:

• Removing the list and allowing all file types
• Removing the list and allowing all file types. Optionally flagging some potentially dangerous extensions in the UI or changing their extension when downloading (for example, .exe becomes .exe.download). I don't like this idea too much, but it's maybe a compromise.

In cases where the extension is used to determine a file type (which, like I said, I'd use it only as a hint), keeping a list internally is fine (for example, to treat images or videos especially). This refers only to the part about blocking files without a pre-approved extensions.

[SebinSong]

Hey @corrideat , Great work on researching/putting together all of these. I'm about to self-assign this one, but It seems like the solution for the issue is pretty much in the description so I feel like why don't you just work on it yourself when you already worked hard to find what to do.. Let me know if you want to take this. (Or just go ahead and re-assign it to yourself)

[SebinSong]

@taoeffect Q1. Ricardo suggested below two as the solutions. What would you like as the solution?

1. Removing the list and allowing all file types
2. Removing the list and allowing all file types. Optionally flagging some potentially dangerous extensions in the UI or changing their extension when downloading (for example, .exe becomes .exe.download). I don't like this idea too much, but it's maybe a compromise.

Q2. if you want 2. as the solution, which file extensions do we include in the list of the 'flagging some potentially dangerous extensions in the UI' other than, exe, js, pdf, 'jar', doc? and also does "flagging in the UI" would mean displaying a modal saying something like, "the file you are attempting to attach could be malicious file..."?

Thanks,

[taoeffect]

@SebinSong

Before we go that route (either route), there's something to check: what does the browser do if you try to drag a macOS app onto it (with an .app extension)? These are essentially folders. In other words, does it allow you to drag .apps? Hopefully the answer is "no".

If folders are not allowed to be uploaded, then I would say go with (1).

[SebinSong]

@taoeffect

does it allow you to drag .apps?

As shared in the slack too, yes it does detect it.