Skip to content
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

An attacker can flood the storage by direct uploading files #13

Closed
choonkeat opened this issue Nov 3, 2015 · 6 comments
Closed

An attacker can flood the storage by direct uploading files #13

choonkeat opened this issue Nov 3, 2015 · 6 comments

Comments

@choonkeat
Copy link
Owner

Though attache presign uploads offers the same protection as S3 direct upload

The pre-signed URLs are valid only for the specified duration.

Within that duration, an attacker can still upload as many files.

To mitigate that, we can adopt the refile and shrine procedure of always uploading to cache then promote to store only when the client app sends a confirmation ping

Current proposal is for /promote to mimic the /delete endpoint

  • require a pre-signed HTTP POST (but valid duration must be pretty short, like < 30s)
  • params include a list of filenames to confirm (batch operation)
  • image server perform promotion async, responds to client app immediately

@janko-m if async promotion fails in the background, what does a shrine user do?

@janko
Copy link

janko commented Nov 3, 2015

Fails in what way? If the attachment is changed on the record before the file is promoted, the stored file is deleted. However, if the record gets deleted, the promotion will error and the stored file won't get deleted, need to fix that.

@choonkeat
Copy link
Owner Author

[async promotion fails] in what way?

When we are using a cloud storage (eg s3), presumably promote is an api call that moves the file from one location to another?

So if this s3 api fail, the file isn't promoted, the cache ultimately gets cleared, then we'll lose a file?

@janko
Copy link

janko commented Nov 3, 2015

Yeah, promote is actually when a cached file is uploaded to "store". Hopefully the background job is set to retry a couple of times, so ultimately it should succeed.

@choonkeat
Copy link
Owner Author

Back when attaché was uploading asynchronously, I've had experience fail scenario before and the retries didn't persist as long as the s3 outage. That incident resulted in some data loss.

For a while we were investigating several methods to make the retry more robust but eventually ended with synchronous upload instead: either the end user experience upload error or the data is safe in s3, no silent data loss.

So for promotion, I'm back to worrying about the failure state again. This picking your brain on this.

If there's no satisfactory solution, I wonder if there's another algorithm to address the original attack vector

@janko
Copy link

janko commented Nov 3, 2015

I think this can be fixed by just setting a long-enough timespan. For example, you make the Sidekiq job retry for 1 day, in increasing periods (e.g. 1st retry in 5 seconds, 2nd is in 15 seconds after last retry, 3rd in a minute etc.), and you also make the cache storage clear out only files that are more than 1 day old. It's statistically impossible that S3 is down for 24 hours.

@choonkeat
Copy link
Owner Author

Guess it'll have to be an option for promote to be sync (slower, zero maintenance) or async (faster, caveat emptor) 🙇

choonkeat added a commit that referenced this issue Dec 14, 2015
- if configured, calling "backup_file" will copy the file from default bucket to a "backup bucket"
- addresses #13 (aka "cache" vs "store" concept in refile)
- reference simplified model of shrinerb/shrine#25 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants