vsivsi / meteor-file-collection

Extends Meteor Collections to handle file data using MongoDB gridFS.
http://atmospherejs.com/vsivsi/file-collection
Other
159 stars 37 forks source link

Is there an option to limit the maximum allowed upload size for a collection somehow? #76

Closed DanielDornhardt closed 8 years ago

DanielDornhardt commented 8 years ago

See title, is there an easy way to limit the maximum upload size somehow (if a malicious client would try to upload a reeealy large amount of fake data chunks for example)?

Could I maybe check the amount of already uploaded data using the deny-rules?

Thanks and best wishes

Daniel

vsivsi commented 8 years ago

Hi, no not currently. There are several ways to upload in this package, and each would need its own mechanism.

For chunked uploads using the built-in resumable.js support, on the client side, you can easily check and reject file uploads that exceed some size inside the fileAdded event: https://github.com/vsivsi/meteor-file-sample-app/blob/master/sample.coffee#L35

However, to prevent a malicious client from using the resumable.js interface to upload huge files will require server-side support in the code that handles such requests. https://github.com/vsivsi/meteor-file-collection/blob/master/src/resumable_server.coffee#L134

It would be easy enough to add a check in that code on resumable.resumableTotalSize to enforce some upper limit on upload sizes on the server side to guard against malicious uploads from untrusted clients.

If you enable plain HTTP POST (or PUT) handling for file uploads, the underlying HTTP request is handled by express.js. This SO question seems to address it... http://stackoverflow.com/questions/13374238/how-to-limit-upload-file-size-in-express-js Because the raw POST/PUT functionality in file-collection seems to be much less popular, I'm not inclined to implement anything for this case.

I'll think about the best way to enable configuring the server-side resumable.js enforcement. Please let me know if you have any thoughts on this.

DanielDornhardt commented 8 years ago

Hi, ok, thanks for your feedback.

I realized now that the users who will be able to upload will at least have to be authenticated or they won't be allowed to create the files in the first place (we have deny rules in place for not-logged-in users).

That would mean that they couldn't upload anything at all using the resumable server code as long as we return "true" from the deny rules for "insert" and "write", right?

That'd be enough for us for the start, because we'll know the customers we'll allow into the app, which makes this issue go away for us at least for now I think.

Thanks and have a great new year!

If I get around to add code to enforce a maximum size for either case I'll definitely post a pull request, but I am afraid I don't have the time at the moment and it's rather unlikely right now :/ .

vsivsi commented 8 years ago

Right, if you require authentication for insert and you can trust authenticated users, then that will prohibit unauthenticated clients from uploading anything, assuming that they also can't see values (typically _id) that can be used to construct a valid URI to HTTP POST to (or the resumable.js equivalent). If that is a possibility in your system, then you should also create a deny rule for write access, so unauthenticated clients also can't overwrite an existing file.

vsivsi commented 8 years ago

@DanielDornhardt I've implemented this on the dev branch with the new maxUploadSize option to the FileCollection constructor. It will enforce the upload size for both resumable.js and ordinary HTTP PUT/POST requests. However, in the latter case, it depends on the Content-length: HTTP header, which can probably be spoofed. The resumable.js support should be considerably more robust, although I'm sure there are probably still ways to exploit it if the attacker is determined enough (there always are...)

Anyway, take a look if you have the chance. I'll probably release this to Atmosphere on Friday.

vsivsi commented 8 years ago

This has been implemented in v1.3.0 on Atmosphere now.

DanielDornhardt commented 8 years ago

Hello Vaughn, thank you for taking the time to sort this out.

I'm over both ears in projects at the moment and didn't find the time yet to write you how much I actually appreciate it.

Thank you for this great package and the amount of work and love you're pouring into it!