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

fix: Restrict upload of binary or unknown file types by default #1507

Merged
merged 26 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
accd80c
Fix #1377
fsbraun Jul 7, 2023
1ec6d3b
Merge branch 'master' of github.com:django-cms/django-filer
fsbraun Jul 10, 2023
50bb9fb
Merge branch 'django-cms:master' into master
fsbraun Jul 12, 2023
f046125
Merge branch 'django-cms:master' into master
fsbraun Jul 14, 2023
d7809dd
Merge branch 'django-cms:master' into master
fsbraun Aug 1, 2023
32770ea
Merge branch 'django-cms:master' into master
fsbraun Aug 4, 2023
01c9f35
Merge branch 'django-cms:master' into master
fsbraun Aug 20, 2023
a625d80
Merge branch 'django-cms:master' into master
fsbraun Sep 6, 2023
556dacb
Merge branch 'django-cms:master' into master
fsbraun Sep 18, 2023
9ed4a06
Merge branch 'django-cms:master' into master
fsbraun Sep 27, 2023
0a534de
Merge branch 'django-cms:master' into master
fsbraun Mar 20, 2024
7f4646b
Merge branch 'django-cms:master' into master
fsbraun Apr 29, 2024
f6cadb9
Merge branch 'django-cms:master' into master
fsbraun May 17, 2024
48e16a6
Merge branch 'django-cms:master' into master
fsbraun Jul 10, 2024
7ab5565
Merge branch 'django-cms:master' into master
fsbraun Aug 9, 2024
4e209c5
Merge branch 'django-cms:master' into master
fsbraun Aug 20, 2024
333d698
Merge branch 'django-cms:master' into master
fsbraun Aug 22, 2024
3a9d5c7
Bump to 3.2.0
fsbraun Aug 22, 2024
e00e3fc
Merge remote-tracking branch 'upstream/master'
fsbraun Sep 18, 2024
e0e7f34
fix: Deny upload of binary or unknown file types
fsbraun Nov 18, 2024
61b23c5
Update tests
fsbraun Nov 18, 2024
b591f03
Update validation.rst
fsbraun Nov 18, 2024
031c1be
Merge branch 'django-cms:master' into fix/restricted-upload
fsbraun Nov 19, 2024
abc251e
Update docs/validation.rst
fsbraun Nov 19, 2024
862bd1b
Add test that binary uploads fail by default
fsbraun Nov 19, 2024
5990ccd
Add migration info
fsbraun Nov 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,7 @@ CHANGELOG


0.5.4a1
=======
========

* Adds description field.

Expand Down
20 changes: 20 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,26 @@ Documentation
Please head over to the separate `documentation <https://django-filer.readthedocs.io/en/latest/index.html>`_
for all the details on how to install, configure and use django-filer.

Upgrading
=========

Version 3.3
-----------

django-filer version 3 contains a change in security policy for file uploads.
**By default, binary file or files of unknown type are not allowed to be uploaded.**
To allow upload of binary files in your project, add

.. code-block:: python

FILER_REMOVE_FILE_VALIDATORS = [
"application/octet-stream",
]

to your project's settings. Be aware that binary files always are a security risk.
See the documentation for more information on how to configure file upload validators,
e.g., running files through a virus checker.


.. |pypi| image:: https://badge.fury.io/py/django-filer.svg
:target: http://badge.fury.io/py/django-filer
Expand Down
48 changes: 43 additions & 5 deletions docs/validation.rst
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ files with the mime type ``image/svg+xml``. Those files are dangerous since
they are executed by a browser without any warnings.

Validation hooks do not restrict the upload of other executable files
(like ``*.exe`` or shell scripts). Those are not automatically executed
(like ``*.exe`` or shell scripts). **Those are not automatically executed
by the browser but still present a point of attack, if a user saves them
to disk and executes them locally.
to disk and executes them locally.**

You can release validation restrictions by setting
``FILER_REMOVE_FILE_VALIDATORS`` to a list of mime types to be removed from
Expand Down Expand Up @@ -111,7 +111,7 @@ This just rejects any file for upload. By default this happens for HTML files

This validator rejects any SVG file that contains the bytes ``<script`` or
``javascript:``. This probably is a too strict criteria, since those bytes
might be part of a legitimate say string. The above code is a simplification
might be part of a legitimate string. The above code is a simplification
the actual code also checks for occurrences of event attribute like
``onclick="..."``.

Expand Down Expand Up @@ -144,10 +144,11 @@ a malicious file unknowingly.
FILER_REMOVE_FILE_VALIDATORS = [
"text/html",
"image/svg+xml",
"application/octet-stream",
]

No HTML upload and restricted SVG upload
........................................
No HTML upload and restricted SVG upload, no binary or unknown file upload
...........................................................................

This is the default setting. It will deny any SVG file that might contain
Javascript. It is prone to false positives (i.e. files being rejected that
Expand Down Expand Up @@ -176,6 +177,8 @@ in the user's browser.
"image/svg+xml": ["filer.validation.deny"],
}

(Still not binary or unknown file upload)

Experimental SVG sanitization
.............................

Expand Down Expand Up @@ -259,3 +262,38 @@ You can use it to distinguish validation for certain user groups if needed.

If you distinguish validation by the mime type, remember to register the
validator function for all relevant mime types.


Checking uploads for viruses using ClamAV
-----------------------------------------

If you have ClamAV installed and use `django-clamd <https://github.com/vstoykov/django-clamd>`_
you can add a validator that checks for viruses in uploaded files.

.. code-block:: python

FILER_REMOVE_FILE_VALIDATORS = ["application/octet-stream"]
FILER_ADD_FILE_VALIDATORS = {
"application/octet-stream": ["my_validator_app.validators.validate_octet_stream"],
}


.. code-block:: python

def validate_octet_stream(file_name: str, file: typing.IO, owner: User, mime_type: str) -> None:
"""Octet streams are binary files without a specific mime type. They are run through
a virus check."""
try:
from django_clamd.validators import validate_file_infection

validate_file_infection(file)
except (ModuleNotFoundError, ImportError):
raise FileValidationError(
_('File "{file_name}": Virus check for binary/unknown file not available').format(file_name=file_name)
)

.. note::

Virus-checked files still might contain executable code. While the code is not
executed by the browser, a user might still download the file and execute it
manually.
Empty file.
1 change: 1 addition & 0 deletions filer/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ def update_server_settings(settings, defaults, s, t):
FILE_VALIDATORS = {
"text/html": ["filer.validation.deny_html"],
"image/svg+xml": ["filer.validation.validate_svg"],
"application/octet-stream": ["filer.validation.deny"],
}

remove_mime_types = getattr(settings, "FILER_REMOVE_FILE_VALIDATORS", [])
Expand Down
22 changes: 22 additions & 0 deletions tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import django
import django.core.files
from django.apps import apps
from django.conf import settings
from django.contrib import admin
from django.contrib.admin import helpers
Expand Down Expand Up @@ -484,6 +485,10 @@ def test_filer_upload_file_no_folder(self, extra_headers={}):
self.assertEqual(stored_image.mime_type, 'image/jpeg')

def test_filer_upload_binary_data(self, extra_headers={}):
config = apps.get_app_config("filer")

validators = config.FILE_VALIDATORS # Remember the validators
config.FILE_VALIDATORS = {} # Remove deny for application/octet-stream
sourcery-ai[bot] marked this conversation as resolved.
Show resolved Hide resolved
self.assertEqual(File.objects.count(), 0)
with open(self.binary_filename, 'rb') as fh:
file_obj = django.core.files.File(fh)
Expand All @@ -494,12 +499,29 @@ def test_filer_upload_binary_data(self, extra_headers={}):
'jsessionid': self.client.session.session_key
}
self.client.post(url, post_data, **extra_headers)
config.FILE_VALIDATORS = validators # Reset validators
fsbraun marked this conversation as resolved.
Show resolved Hide resolved

self.assertEqual(Image.objects.count(), 0)
self.assertEqual(File.objects.count(), 1)
stored_file = File.objects.first()
self.assertEqual(stored_file.original_filename, self.binary_name)
self.assertEqual(stored_file.mime_type, 'application/octet-stream')

def test_filer_upload_binary_data_fails_by_default(self, extra_headers={}):
self.assertEqual(File.objects.count(), 0)
with open(self.binary_filename, 'rb') as fh:
file_obj = django.core.files.File(fh)
url = reverse('admin:filer-ajax_upload')
post_data = {
'Filename': self.binary_name,
'Filedata': file_obj,
'jsessionid': self.client.session.session_key
}
self.client.post(url, post_data, **extra_headers)

self.assertEqual(Image.objects.count(), 0)
self.assertEqual(File.objects.count(), 0)

def test_filer_ajax_upload_file(self):
self.assertEqual(Image.objects.count(), 0)
folder = Folder.objects.create(name='foo')
Expand Down
Loading