5 ms·
It's still far short of being suitable for use of any private documents. https://github.com/baicunko/scanyourpdf/blob/master/pdfwebsite/views.py#L45 https://gi
by lewiscollard 6y ago
It's still far short of being suitable for use of any private documents.
https://github.com/baicunko/scanyourpdf/blob/master/pdfwebsite/views.py#L45 https://github.com/baicunko/scanyourpdf/blob/master/pdfwebsi...
This is rather less than secure; output files are named, e.g., "Scan_2020512_{four random lower-case letters}.pdf" into a web-server-readable directory.
That gives a total of 456976 different possible filenames on a day. It's more than feasible to brute-force that many filenames in the hour before files get deleted.
OP: I don't think randomly-suffixed file names are an inherently bad way to approach this. But you should definitely consider using a longer random string, and definitely consider not using the `random` module too (it is not secure and is not intended to be).
- baicunko 6y agoThank you for the comments. I agree with you, I will decrease how long the file is in the server (I just hit 40gb from hacker news) as well as implement rate limiting to prevent any brute force
- lewiscollard 6y agoRate limiting (if by that you mean at the firewall or the web server) is not the way to do it. That shifts the problem somewhere else in the stack, into a place that isn't under version control in the same repository. Consider: If you moved this on to another server, would you remember to enable rate limiting there? If someone else uses your code, will they know to enable rate limiting? Rate limiting isn't a bad idea, but your security should not depend on it, especially as you have a way of securing it in your application. base64.b16encode(os.urandom(8)) will give you a 64-bit, filename-safe, as-close-to-random-as-reasonable suffix that should be long enough to make it brute-force-proof :) The same reasoning applies to the cron job (I presume) that is cleaning your files - that's something you have to remember to set up for future (re-)deployments. Edit: I'd also like to add that showing your code on HN takes bravery and this is, in fact, a neat tool that solves a problem I really wish didn't exist. So, good work on both counts :)
- OriginalPenguin 6y agoOr use a uuid4 for the suffix.
- bwindsor 6y agoOr use secrets.token_urlsafe https://docs.python.org/3/library/secrets.html#secrets.token_urlsafe https://docs.python.org/3/library/secrets.html#secrets.token...
- lewiscollard 6y agoHey thanks - somehow, I entirely missed this being a Thing in the stdlib in 3. :) I've used it in my PR on baicunko's repo.
- baicunko 6y agothanks! took me quite a while to prepare as I read a bunch of other servers failing catastrophically when posting on HN due to the sheer amount of traffic. I will start working on your comments throughout the weekend, I agree with most of them. Would love for you to follow the github page for any other comments you may have, all are appreciated
- lewiscollard 6y agoIt was while reading my above comments that I realised I should have shut up and contributed code instead, because that's definitely more helpful than being critical on HN, especially to a newcomer & their first project. So that is what I've decided to do! First step: a PR coming out of getting this up and running on my Ubuntu box. :)
- baicunko 6y agoNo worries I think it's great that everyone is voicing their comments. Also, thanks for the PR, will check them later once everything is more normal!
- mahathu 6y ago
- tinix 6y agohttps://docs.python.org/3/library/tempfile.html https://docs.python.org/3/library/tempfile.html >>> tempfile.mkstemp(suffix='.pdf') (4, '/tmp/tmp0g7l3uq7.pdf') one could take it a step further... >>> tempfile.mkstemp(suffix='.pdf', prefix=uuid.uuid4().hex) (5, '/tmp/7b7881400a2348bfae63d37b970d4489pqhz_1g2.pdf')