django-cms / django-filer

File and Image Management Application for django
https://django-filer.readthedocs.io/
Other
1.76k stars 577 forks source link

feat: Introduce upload validation to prevent XSS exploits with SVG #1364

Closed fsbraun closed 1 year ago

fsbraun commented 1 year ago

Description

This PR introduces upload validation hooks to prevent users from intentionally or unintentionally upload malicious SVG or HTML files.

The default setting is to prevent all HTML file uploads and to check for potential scripts in SVG files.

To keep things simple, the SVG file check looks for certain keywords or obfuscation tactics. IMHO, it will lean towards false positives (i.e. rejecting files that in fact are OK).

The default validator can be replaced by a more sophisticated custom validator. However, since bleach is sunset, it is a non-trivial task to create one.

Related resources

Checklist

codecov[bot] commented 1 year ago

Codecov Report

Merging #1364 (f318b37) into master (deafdc2) will increase coverage by 0.35%. The diff coverage is 82.35%.

@@            Coverage Diff             @@
##           master    #1364      +/-   ##
==========================================
+ Coverage   73.26%   73.61%   +0.35%     
==========================================
  Files          73       74       +1     
  Lines        3336     3400      +64     
  Branches      542      551       +9     
==========================================
+ Hits         2444     2503      +59     
- Misses        720      726       +6     
+ Partials      172      171       -1     
Impacted Files Coverage Δ
filer/utils/pil_exif.py 28.57% <0.00%> (ø)
filer/validation.py 78.94% <78.94%> (ø)
filer/apps.py 90.90% <87.50%> (-9.10%) :arrow_down:
filer/admin/clipboardadmin.py 94.73% <100.00%> (+0.61%) :arrow_up:
filer/admin/folderadmin.py 72.78% <100.00%> (ø)
filer/settings.py 80.19% <100.00%> (+0.40%) :arrow_up:

... and 1 file with indirect coverage changes

fsbraun commented 1 year ago

@marksweb I just implemented the requested changes and added another setting: FILER_MIME_TYPE_WHITELIST which would allow to whitelist accepted mime types, effectively barring all others. So, one could restrict filer to only accept images and no other formats (including all thinkable executable files)

Tests are also in now.

fsbraun commented 1 year ago

@jrief pointed me to reportlab's ability to rewrite SVG files, thereby eliminating script tags and event attributes. I added this as an experimental feature.