Skip to content

Commit

Permalink
fix: decompression bomb attack in the Filer (#1426)
Browse files Browse the repository at this point in the history
  • Loading branch information
vinitkumar authored Sep 28, 2023
1 parent 8293ba1 commit 964f48d
Show file tree
Hide file tree
Showing 78 changed files with 2,943 additions and 1,849 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,18 @@ CHANGELOG
unreleased
==========

* feat: limit uploaded image area (width x height) to prevent decompression
bombs
* fix: Run validators on updated files in file change view
* fix: Update mime type if uploading file in file change view
* fix: Do not allow to remove the file field from an uplaoded file in
the admin interface
* fix: refactor upload checks into running validators in the admin
and adding clean methods for file and (abstract) image models.
* Fixed two more instances of javascript int overflow issue (#1335)
* fix: ensure uniqueness of icon admin url names
* fix: Crash with django-storage if filer file does not have a
storage file attached

3.0.6 (2023-09-08)
==================
Expand Down
13 changes: 13 additions & 0 deletions docs/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,19 @@ Limits the maximal file size if set. Takes an integer (file size in MB).

Defaults to ``None``.

``FILER_MAX_IMAGE_PIXELS``
--------------------------------

Limits the maximal pixel size of the image that can be uploaded to the Filer.
It will also be lower than or equals to the MAX_IMAGE_PIXELS that Pillow's PIL allows.


``MAX_IMAGE_PIXELS = int(1024 * 1024 * 1024 // 4 // 3)``

Defaults to ``MAX_IMAGE_PIXELS``. But when set, should always be lower than the MAX_IMAGE_PIXELS limit set by Pillow.

This is useful setting to prevent decompression bomb DOS attack.


``FILER_ADD_FILE_VALIDATORS``
-----------------------------
Expand Down
70 changes: 38 additions & 32 deletions filer/admin/clipboardadmin.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from django.contrib import admin, messages
from django.core.exceptions import ValidationError
from django.forms.models import modelform_factory
from django.http import JsonResponse
from django.urls import path
Expand All @@ -9,7 +10,7 @@
from ..models import Clipboard, ClipboardItem, Folder
from ..utils.files import handle_request_files_upload, handle_upload
from ..utils.loader import load_model
from ..validation import FileValidationError, validate_upload
from ..validation import validate_upload
from . import views


Expand Down Expand Up @@ -112,48 +113,53 @@ def ajax_upload(request, folder_id=None):
break
uploadform = FileForm({'original_filename': filename, 'owner': request.user.pk},
{'file': upload})
uploadform.request = request
uploadform.instance.mime_type = mime_type
if uploadform.is_valid():
try:
validate_upload(filename, upload, request.user, mime_type)
except FileValidationError as error:
from django.contrib.messages import ERROR, add_message
message = str(error)
add_message(request, ERROR, message)
return JsonResponse({'error': message})
file_obj = uploadform.save(commit=False)
# Enforce the FILER_IS_PUBLIC_DEFAULT
file_obj.is_public = filer_settings.FILER_IS_PUBLIC_DEFAULT
file_obj = uploadform.save(commit=False)
# Enforce the FILER_IS_PUBLIC_DEFAULT
file_obj.is_public = filer_settings.FILER_IS_PUBLIC_DEFAULT
except ValidationError as error:
messages.error(request, str(error))
return JsonResponse({'error': str(error)})
file_obj.folder = folder
file_obj.save()
# TODO: Deprecated/refactor
# clipboard_item = ClipboardItem(
# clipboard=clipboard, file=file_obj)
# clipboard_item.save()

thumbnail = None
data = {
'thumbnail': thumbnail,
'alt_text': '',
'label': str(file_obj),
'file_id': file_obj.pk,
}
# prepare preview thumbnail
if isinstance(file_obj, Image):
thumbnail_180_options = {
'size': (180, 180),
'crop': True,
'upscale': True,
try:
thumbnail = None
data = {
'thumbnail': thumbnail,
'alt_text': '',
'label': str(file_obj),
'file_id': file_obj.pk,
}
thumbnail_180 = file_obj.file.get_thumbnail(
thumbnail_180_options)
data['thumbnail_180'] = thumbnail_180.url
data['original_image'] = file_obj.url
return JsonResponse(data)
# prepare preview thumbnail
if isinstance(file_obj, Image):
thumbnail_180_options = {
'size': (180, 180),
'crop': True,
'upscale': True,
}
thumbnail_180 = file_obj.file.get_thumbnail(
thumbnail_180_options)
data['thumbnail_180'] = thumbnail_180.url
data['original_image'] = file_obj.url
return JsonResponse(data)
except Exception as error:
messages.error(request, str(error))
return JsonResponse({"error": str(error)})
else:
form_errors = '; '.join(['{}: {}'.format(
field,
', '.join(errors)) for field, errors in list(
uploadform.errors.items())
for key, error_list in uploadform.errors.items():
for error in error_list:
messages.error(request, error)

form_errors = '; '.join(['{}'.format(
', '.join(errors)) for errors in list(uploadform.errors.values())
])
return JsonResponse({'message': str(form_errors)}, status=422)
return JsonResponse({'error': str(form_errors)}, status=200)
26 changes: 25 additions & 1 deletion filer/admin/fileadmin.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import mimetypes

from django import forms
from django.contrib.admin.utils import unquote
from django.contrib.staticfiles.storage import staticfiles_storage
Expand All @@ -8,6 +10,7 @@
from django.utils.timezone import now
from django.utils.translation import gettext as _

from easy_thumbnails.engine import NoSourceGenerator
from easy_thumbnails.exceptions import InvalidImageFormatError
from easy_thumbnails.files import get_thumbnailer
from easy_thumbnails.models import Thumbnail as EasyThumbnail
Expand All @@ -25,6 +28,27 @@ class Meta:
model = File
exclude = ()

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.fields["file"].widget = forms.FileInput()

def clean(self):
from ..validation import validate_upload
cleaned_data = super().clean()
if "file" in self.changed_data and cleaned_data["file"]:
mime_type = mimetypes.guess_type(cleaned_data["file"].name)[0] or 'application/octet-stream'
file = cleaned_data["file"]
file.open("w+") # Allow for sanitizing upload
file.seek(0)
validate_upload(
file_name=cleaned_data["file"].name,
file=file.file,
owner=cleaned_data["owner"],
mime_type=mime_type,
)
file.open("r")
return self.cleaned_data


class FileAdmin(PrimitivePermissionAwareModelAdmin):
list_display = ('label',)
Expand Down Expand Up @@ -185,7 +209,7 @@ def icon_view(self, request, file_id: int, size: int) -> HttpResponse:
# Touch thumbnail to allow it to be prefetched for directory listing
EasyThumbnail.objects.filter(name=thumbnail.name).update(modified=now())
return HttpResponseRedirect(thumbnail.url)
except InvalidImageFormatError:
except (InvalidImageFormatError, NoSourceGenerator):
return HttpResponseRedirect(staticfiles_storage.url('filer/icons/file-missing.svg'))


Expand Down
8 changes: 4 additions & 4 deletions filer/admin/imageadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
from ..thumbnail_processors import normalize_subject_location
from ..utils.compatibility import string_concat
from ..utils.loader import load_model
from .fileadmin import FileAdmin
from .fileadmin import FileAdmin, FileAdminChangeFrom


Image = load_model(FILER_IMAGE_MODEL)


class ImageAdminForm(forms.ModelForm):
class ImageAdminForm(FileAdminChangeFrom):
subject_location = forms.CharField(
max_length=64, required=False,
label=_('Subject location'),
Expand Down Expand Up @@ -58,8 +58,8 @@ def clean_subject_location(self):
err_code = 'invalid_subject_format'

elif (
coordinates[0] > self.instance.width
or coordinates[1] > self.instance.height
coordinates[0] > self.instance.width > 0
or coordinates[1] > self.instance.height > 0
):
err_msg = gettext_lazy(
'Subject location is outside of the image. ')
Expand Down
6 changes: 3 additions & 3 deletions filer/fields/multistorage_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class MultiStorageFileDescriptor(FileDescriptor):
"""
This is rather similar to Django's ImageFileDescriptor.
It calls <field name>_data_changed on model instance when new
value is set. The callback is suposed to update fields which
value is set. The callback is supposed to update fields which
are related to file data (like size, checksum, etc.).
When this is called from model __init__ (prev_assigned=False),
it does nothing because related fields might not have values yet.
Expand All @@ -58,7 +58,7 @@ def __set__(self, instance, value):
# To prevent recalculating file data related attributes when we are instantiating
# an object from the database, update only if the field had a value before this assignment.
# To prevent recalculating upon reassignment of the same file, update only if value is
# different than the previous one.
# different from the previous one.
if prev_assigned and value != previous_file:
callback_attr = f'{self.field.name}_data_changed'
if hasattr(instance, callback_attr):
Expand Down Expand Up @@ -123,7 +123,7 @@ def exists(self):
"""
Returns ``True`` if underlying file exists in storage.
"""
return self.storage.exists(self.name)
return self.name and self.storage.exists(self.name)


class MultiStorageFileField(easy_thumbnails_fields.ThumbnailerField):
Expand Down
Binary file modified filer/locale/bg/LC_MESSAGES/django.mo
Binary file not shown.
Loading

0 comments on commit 964f48d

Please sign in to comment.