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

[OPINION/share yours] Should we limit file types in the file uploader? #45

Closed
robiso opened this Issue Sep 21, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@robiso
Owner

robiso commented Sep 21, 2017

Short discussion description

  • Since version 2.3.0, the built in WonderCMS file uploader accepts ANY file type, not just pictures.
  • This brings great responsibility to the admin user. The admin user must NOT upload malicious files that could harm their own website.
  • Some people have voiced their opinion that this is bad and that the user could harm himself by uploading any malicious files.

Below are arguments for and against this uploading any file type feature

Arguments to keep this feature (uploading any files)

  • This is similar functionality to FTP/SSH uploading (no limits).
  • Only the admin can upload files:
    • meaning if the admin wants to hurt his/her website, there are other channels to do self-harm than the simple file uploader.
  • We've removed this limit to enable users to upload whatever extension they want.

Arguments to limit this feature to just uploading pictures

  • User can not inflict harm to their website through the file uploader (but can still do this via FTP/SSH/other channels).
  • If the user gets compromised, an attacker can upload whatever they want in their files directory.

How can the user be compromised?

  • Sharing their password.
  • Sharing their login URL, which could lead to password brute forcing.
    • It is very important to keep your login URL safe for cases like this.

What happens when an user in a shared environment gets compromised?

  • Shared hosting plans usually have a protection in place, a protection that limits the user only to their account - so no further damager could be done (to other users on the same hosting plan):
    • the user inflicts damage only upon himself and his website (and no other users on the same shared hosting plan).

@robiso robiso added the discussion label Sep 21, 2017

@robiso robiso changed the title from [OPINION/share yours] Should we limit what files we should upload? to [OPINION/share yours] Should we limit file types in the file uploader? Sep 21, 2017

@anolis

This comment has been minimized.

Contributor

anolis commented Oct 9, 2017

I think this is a good consideration to take, I feel that perhaps a default "accepted file type" list would be low cost to size and easily modifiable if the admin feels the desire to upload other filetypes after install.

@robiso

This comment has been minimized.

Owner

robiso commented Oct 10, 2017

Thank you for your input @anolis, always appreciated.

A default accepted file type list is indeed low cost to size. As far as I understand your input, you are FOR a short list of allowed file-types, which can be modified later by the admin with a functions.php file, if they wanted to allow more file-types?

@anolis

This comment has been minimized.

Contributor

anolis commented Oct 10, 2017

Yes, exactly thumbs up to that.

@robiso

This comment has been minimized.

Owner

robiso commented Oct 28, 2017

Thank you @anolis.

Asking for additional opinions and would like to later close this issue and apply the general opinion.

  • share your opinion with a comment below and shape the future of WonderCMS
@Tangol

This comment has been minimized.

Tangol commented Nov 27, 2017

I also agree with @anolis
That would be the best solution.

@robiso

This comment has been minimized.

Owner

robiso commented Nov 27, 2017

I will self assign on this issue.

How does this list seem to you guys as the "default accepted" file types?

Edited list is a couple of posts lower

@robiso robiso self-assigned this Nov 27, 2017

@Tangol

This comment has been minimized.

Tangol commented Nov 27, 2017

My list:

gif, jpg, jpeg, png, svg, ico
flv, mp3, mp4, m4a, wmv, mpg, mov, avi, mkv, webm, ogg, ogv
txt, doc, docx, ppt, pptx, xls, xlsx, odt, ods, pdf, psd
zip, rar

@robiso

This comment has been minimized.

Owner

robiso commented Nov 27, 2017

Thanks @Tangol!

Full list with added svg, flv, mkv, webm, ogg, ogv, rar, txt, kdbx and ods, alongside with their mime types.

$allowed = [
	'doc' => 'application/vnd.ms-word',
	'docx' => 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
	'kdbx' => 'application/octet-stream',
	'ods' => 'application/vnd.oasis.opendocument.spreadsheet',
	'odt' => 'application/vnd.oasis.opendocument.text',
	'ogg' => 'application/ogg',
	'pdf' => 'application/pdf',
	'ppt' => 'application/vnd.ms-powerpoint,
	'pptx' => 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
	'psd' => 'application/photoshop',
	'rar' => 'application/rar',
	'xls' => 'application/vnd.ms-excel',
	'xlsx' => 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
	'zip' => 'application/zip',
	'm4a' => 'audio/mp4',
	'mp3' => 'audio/mpeg',
	'gif' => 'image/gif',
	'jpg' => 'image/jpeg',
	'ico' => 'image/x-icon',
	'png' => 'image/png',
	'svg' => 'image/svg+xml',
	'txt' => 'text/plain',
	'avi' => 'video/avi',
	'flv' => 'video/x-flv',
	'mkv' => 'video/x-matroska',
	'mov' => 'video/quicktime',
	'mp4' => 'video/mp4',
	'mpg' => 'video/mpeg',
	'ogv' => 'video/ogg',
	'webm' => 'video/webm',
	'wmv' => 'video/x-ms-wmv'
];

Any other suggestions you'd like to see on the list above?

I'll be implementing this allow list with the mentioned release.

@robiso

This comment has been minimized.

Owner

robiso commented Jan 1, 2018

Closing this issue as we have implemented this list with today's 2.4.0 version.

@robiso robiso closed this Jan 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment