-
-
Notifications
You must be signed in to change notification settings - Fork 775
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
Validate uploaded file type and size #4479
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4479 +/- ##
==========================================
- Coverage 30.08% 30.07% -0.02%
==========================================
Files 209 209
Lines 16850 16867 +17
Branches 2267 2271 +4
==========================================
+ Hits 5070 5072 +2
- Misses 11582 11597 +15
Partials 198 198
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #4479 +/- ##
==========================================
- Coverage 30.04% 30.02% -0.02%
==========================================
Files 209 209
Lines 16864 16881 +17
Branches 2272 2276 +4
==========================================
+ Hits 5067 5069 +2
- Misses 11601 11616 +15
Partials 196 196
Continue to review full report at Codecov.
|
'message': 'No File Found' | ||
} | ||
elif uploaded_file.size > 31457280: | ||
# 30MB max file size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would 30 MB be too much?
Eh, I just picked a number that I think is not too high and not too low -
PDFs can be suprisingly big sometimes but more than 30 MB or so and I think
we could open up an attack vector.
…-Dan Lipert
On Wed, May 29, 2019 at 8:33 PM Aditya Anand M C ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/dashboard/views.py
<#4479 (comment)>:
> @@ -1777,6 +1783,29 @@ def profile_job_opportunity(request, handle):
return JsonResponse(response)
+def invalid_file_response(uploaded_file, supported):
+ response = None
+ if not uploaded_file:
+ response = {
+ 'status': 400,
+ 'message': 'No File Found'
+ }
+ elif uploaded_file.size > 31457280:
+ # 30MB max file size
would 30 MB be too much?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#4479?email_source=notifications&email_token=AAGUHEQ6UTVOOV4XUKCJTKTPXZSXZA5CNFSM4HPNTYF2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZ7OS3I#pullrequestreview-243198317>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGUHERRNRNDZYEZ2SSG5H3PXZSXZANCNFSM4HPNTYFQ>
.
|
Description
This PR checks uploaded files' type directly via libmagic. This is useful to narrow the potential attack vectors due to accepting user-uploaded files.
Refers/Fixes
#4432
Testing
Tested both the profile resume upload and NDA upload and reupload forms by submitting invalid files that satisfied the client-side file type (file extension) checks. Response from server was as expected with appropriate alert shades. I then manually submitted the correct form of file and was able to create a bounty, apply to work, submit work, and payout. I tested with PDF, DOCX and DOC files for the NDA.