Skip to content

Conversation

@androolloyd
Copy link
Contributor

Description
- Adding a check to the `invalid_file_response` helper method to ensure that <script> tags aren't defined inside the content of the uploaded file buffer for uploaded user content.
Refers/Fixes

#5245

Testing

Tested through onboarding, and through /settings/job

	- Adding a check to ensure that <script> tags aren't defined inside the content of the uploaded file buffer for uploaded user content.
@androolloyd
Copy link
Contributor Author

a solution that works with file chunks would work better here instead of loading the entire buffer into memory at one time, I tried a few different approaches with mmap, save having to write the files to the hard disk before loading I haven't been able to solve it using that approach(chunks + mmap)

@codecov
Copy link

codecov bot commented Oct 26, 2019

Codecov Report

Merging #5394 into master will increase coverage by 2.15%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #5394      +/-   ##
=========================================
+ Coverage   29.85%     32%   +2.15%     
=========================================
  Files         241     242       +1     
  Lines       20406   23728    +3322     
  Branches     2918    3939    +1021     
=========================================
+ Hits         6092    7594    +1502     
- Misses      14063   15777    +1714     
- Partials      251     357     +106
Impacted Files Coverage Δ
app/dashboard/views.py 14.51% <0%> (+1.55%) ⬆️
app/app/urls.py 85% <0%> (-5%) ⬇️
app/quests/views.py 22.09% <0%> (-1.72%) ⬇️
app/quests/helpers.py 20.48% <0%> (-0.76%) ⬇️
app/app/context.py 47.45% <0%> (-0.7%) ⬇️
app/grants/views.py 15.89% <0%> (-0.38%) ⬇️
app/marketing/views.py 11.51% <0%> (-0.27%) ⬇️
app/retail/utils.py 9.75% <0%> (-0.18%) ⬇️
app/dashboard/helpers.py 13.84% <0%> (-0.14%) ⬇️
...perftools/management/commands/create_page_cache.py 0% <0%> (ø) ⬆️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b95c8f...1c309a1. Read the comment docs.

@thelostone-mc
Copy link
Contributor

Oh this is sweet :D

… instead of processing the entire binary value at once
@androolloyd
Copy link
Contributor Author

androolloyd commented Oct 28, 2019

I have updated the approach, to leverage chunks and to include a forbidden content array that can be populated/managed to ensure that content that would violate terms of service or otherwise cause harm can be scrubbed and denied, note this doesn't use mmap as the uploadedfiles are InMemory objects anyways, we can just handle the raw binary and iterate and search.

@androolloyd androolloyd changed the title Stored XSS WIP Stored XSS - [ In Review ] Oct 28, 2019
@danlipert danlipert merged commit ce3dc30 into master Nov 6, 2019
@androolloyd androolloyd deleted the bug/stored-xss-5245 branch November 6, 2019 15:03
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

Successfully merging this pull request may close these issues.

5 participants