diff --git a/src/pretix/api/views/upload.py b/src/pretix/api/views/upload.py index 767d427bbb..bd75a7d982 100644 --- a/src/pretix/api/views/upload.py +++ b/src/pretix/api/views/upload.py @@ -21,6 +21,7 @@ # import datetime +from django.core.exceptions import ValidationError as DjangoValidationError from django.utils.timezone import now from oauth2_provider.contrib.rest_framework import OAuth2Authentication from rest_framework.authentication import SessionAuthentication @@ -33,6 +34,9 @@ from pretix.api.auth.device import DeviceTokenAuthentication from pretix.api.auth.permission import AnyAuthenticatedClientPermission from pretix.api.auth.token import TeamTokenAuthentication from pretix.base.models import CachedFile +from pretix.helpers.images import ( + IMAGE_TYPES, validate_uploaded_file_for_valid_image, +) ALLOWED_TYPES = { 'image/gif': {'.gif'}, @@ -61,6 +65,13 @@ class UploadView(APIView): name=file_obj.name, type=content_type )) + + if content_type in IMAGE_TYPES: + try: + validate_uploaded_file_for_valid_image(file_obj) + except DjangoValidationError as e: + raise ValidationError(e.message) + cf = CachedFile.objects.create( expires=now() + datetime.timedelta(days=1), date=now(), diff --git a/src/pretix/base/forms/questions.py b/src/pretix/base/forms/questions.py index 9fcc7342e7..e6419fe758 100644 --- a/src/pretix/base/forms/questions.py +++ b/src/pretix/base/forms/questions.py @@ -531,7 +531,7 @@ class PortraitImageField(SizeValidationMixin, ExtValidationMixin, forms.FileFiel code='aspect_ratio_not_3_by_4', ) except Exception as exc: - logger.exception('foo') + logger.exception('Could not parse image') # Pillow doesn't recognize it as an image. if isinstance(exc, ValidationError): raise diff --git a/src/pretix/base/models/items.py b/src/pretix/base/models/items.py index fed9e29f7f..f90b9eec02 100644 --- a/src/pretix/base/models/items.py +++ b/src/pretix/base/models/items.py @@ -62,6 +62,7 @@ from pretix.base.models.base import LoggedModel from pretix.base.models.fields import MultiStringField from pretix.base.models.tax import TaxedPrice +from ...helpers.images import ImageSizeValidator from .event import Event, SubEvent @@ -429,7 +430,8 @@ class Item(LoggedModel): picture = models.ImageField( verbose_name=_("Product picture"), null=True, blank=True, max_length=255, - upload_to=itempicture_upload_to + upload_to=itempicture_upload_to, + validators=[ImageSizeValidator()] ) available_from = models.DateTimeField( verbose_name=_("Available from"), diff --git a/src/pretix/control/forms/__init__.py b/src/pretix/control/forms/__init__.py index c1196c3d8a..091c087cd5 100644 --- a/src/pretix/control/forms/__init__.py +++ b/src/pretix/control/forms/__init__.py @@ -51,6 +51,9 @@ from django_scopes.forms import SafeModelMultipleChoiceField from pretix.helpers.hierarkey import clean_filename from ...base.forms import I18nModelForm +from ...helpers.images import ( + IMAGE_EXTS, validate_uploaded_file_for_valid_image, +) # Import for backwards compatibility with okd import paths from ...base.forms.widgets import ( # noqa @@ -220,6 +223,10 @@ class ExtValidationMixin: ext = ext.lower() if ext not in self.ext_whitelist: raise forms.ValidationError(_("Filetype not allowed!")) + + if ext in IMAGE_EXTS: + validate_uploaded_file_for_valid_image(data) + return data diff --git a/src/pretix/helpers/images.py b/src/pretix/helpers/images.py new file mode 100644 index 0000000000..c347bb9f9d --- /dev/null +++ b/src/pretix/helpers/images.py @@ -0,0 +1,85 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# +import logging +from io import BytesIO + +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ +from PIL.Image import MAX_IMAGE_PIXELS, DecompressionBombError + +IMAGE_TYPES = {'image/gif', 'image/jpeg', 'image/png'} +IMAGE_EXTS = {'.gif', '.jpg', '.jpeg', '.png'} + + +logger = logging.getLogger(__name__) + + +def validate_uploaded_file_for_valid_image(f): + if f is None: + return None + + from PIL import Image + + # We need to get a file object for Pillow. We might have a path or we might + # have to read the data into memory. + if hasattr(f, 'temporary_file_path'): + file = f.temporary_file_path() + else: + if hasattr(f, 'read'): + file = BytesIO(f.read()) + else: + file = BytesIO(f['content']) + + try: + try: + image = Image.open(file) + # verify() must be called immediately after the constructor. + image.verify() + except DecompressionBombError: + raise ValidationError(_( + "The file you uploaded has a very large number of pixels, please upload a picture with smaller dimensions." + )) + + # load() is a potential DoS vector (see Django bug #18520), so we verify the size first + if image.width * image.height > MAX_IMAGE_PIXELS: + raise ValidationError(_( + "The file you uploaded has a very large number of pixels, please upload a picture with smaller dimensions." + )) + except Exception as exc: + logger.exception('Could not parse image') + # Pillow doesn't recognize it as an image. + if isinstance(exc, ValidationError): + raise + raise ValidationError(_( + "Upload a valid image. The file you uploaded was either not an image or a corrupted image." + )) from exc + if hasattr(f, 'seek') and callable(f.seek): + f.seek(0) + + +class ImageSizeValidator: + def __call__(self, image): + if image.width * image.height > MAX_IMAGE_PIXELS: + raise ValidationError(_( + "The file you uploaded has a very large number of pixels, please upload a picture with smaller dimensions." + )) + return image diff --git a/src/pretix/testutils/api.py b/src/pretix/testutils/api.py index 2d5d1ce3a8..a0e19255bc 100644 --- a/src/pretix/testutils/api.py +++ b/src/pretix/testutils/api.py @@ -29,4 +29,4 @@ class UploadRenderer(BaseRenderer): def render(self, data, accepted_media_type=None, renderer_context=None): self.media_type = data['media_type'] - return data['file'] + return data['file'].read() diff --git a/src/tests/api/test_cart.py b/src/tests/api/test_cart.py index 21af7788bd..8849d64484 100644 --- a/src/tests/api/test_cart.py +++ b/src/tests/api/test_cart.py @@ -29,6 +29,7 @@ from django.core.files.base import ContentFile from django.utils.timezone import now from django_scopes import scopes_disabled from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.base.models import Question, SeatingPlan from pretix.base.models.orders import CartPosition @@ -467,7 +468,7 @@ def test_cartpos_create_answer_validation(token_client, organizer, event, item, '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', diff --git a/src/tests/api/test_checkin.py b/src/tests/api/test_checkin.py index 9132e9af89..6d76e13a80 100644 --- a/src/tests/api/test_checkin.py +++ b/src/tests/api/test_checkin.py @@ -32,6 +32,7 @@ from django_countries.fields import Country from django_scopes import scopes_disabled from i18nfield.strings import LazyI18nString from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.api.serializers.item import QuestionSerializer from pretix.base.models import ( @@ -1057,7 +1058,7 @@ def test_question_upload(token_client, organizer, clist, event, order, question) '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', diff --git a/src/tests/api/test_checkinrpc.py b/src/tests/api/test_checkinrpc.py index ce908d32cf..8a9e93efe7 100644 --- a/src/tests/api/test_checkinrpc.py +++ b/src/tests/api/test_checkinrpc.py @@ -30,6 +30,7 @@ from django_countries.fields import Country from django_scopes import scopes_disabled from i18nfield.strings import LazyI18nString from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.api.serializers.item import QuestionSerializer from pretix.base.models import Checkin, InvoiceAddress, Order, OrderPosition @@ -563,7 +564,7 @@ def test_question_upload(token_client, organizer, clist, event, order, question) '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', diff --git a/src/tests/api/test_events.py b/src/tests/api/test_events.py index a043cde612..6745d4423d 100644 --- a/src/tests/api/test_events.py +++ b/src/tests/api/test_events.py @@ -44,6 +44,7 @@ from django.utils.timezone import now from django_countries.fields import Country from django_scopes import scopes_disabled from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.base.models import ( Event, InvoiceAddress, Order, OrderPosition, Organizer, SeatingPlan, @@ -1351,7 +1352,7 @@ def test_patch_event_settings_file(token_client, organizer, event): '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', @@ -1363,7 +1364,7 @@ def test_patch_event_settings_file(token_client, organizer, event): '/api/v1/upload', data={ 'media_type': 'application/pdf', - 'file': ContentFile('file.pdf', 'invalid pdf content') + 'file': ContentFile('invalid pdf content') }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.pdf"', diff --git a/src/tests/api/test_items.py b/src/tests/api/test_items.py index d3def769d4..7f96d9365e 100644 --- a/src/tests/api/test_items.py +++ b/src/tests/api/test_items.py @@ -43,6 +43,7 @@ from django.core.files.base import ContentFile from django_countries.fields import Country from django_scopes import scopes_disabled from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.base.channels import get_all_sales_channels from pretix.base.models import ( @@ -1119,7 +1120,7 @@ def test_item_file_upload(token_client, organizer, event, item): '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', @@ -1143,7 +1144,7 @@ def test_item_file_upload(token_client, organizer, event, item): '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', diff --git a/src/tests/api/test_order_change.py b/src/tests/api/test_order_change.py index 7b4877f1b5..da78eba7a7 100644 --- a/src/tests/api/test_order_change.py +++ b/src/tests/api/test_order_change.py @@ -31,6 +31,7 @@ from django.utils.timezone import now from django_countries.fields import Country from django_scopes import scopes_disabled from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.base.models import ( InvoiceAddress, Order, OrderPosition, Question, SeatingPlan, @@ -1018,7 +1019,7 @@ def test_position_update_question_handling(token_client, organizer, event, order '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', diff --git a/src/tests/api/test_order_create.py b/src/tests/api/test_order_create.py index eedc77b615..4cc1273179 100644 --- a/src/tests/api/test_order_create.py +++ b/src/tests/api/test_order_create.py @@ -32,6 +32,7 @@ from django.utils.timezone import now from django_countries.fields import Country from django_scopes import scopes_disabled from pytz import UTC +from tests.const import SAMPLE_PNG from pretix.base.models import ( InvoiceAddress, Order, OrderPosition, Question, SeatingPlan, @@ -1424,7 +1425,7 @@ def test_order_create_answer_validation(token_client, organizer, event, item, qu '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', diff --git a/src/tests/api/test_organizers.py b/src/tests/api/test_organizers.py index 576d486361..bb1123931d 100644 --- a/src/tests/api/test_organizers.py +++ b/src/tests/api/test_organizers.py @@ -21,6 +21,7 @@ # import pytest from django.core.files.base import ContentFile +from tests.const import SAMPLE_PNG from pretix.testutils.mock import mocker_context @@ -130,7 +131,7 @@ def test_patch_organizer_settings_file(token_client, organizer): '/api/v1/upload', data={ 'media_type': 'image/png', - 'file': ContentFile('file.png', 'invalid png content') + 'file': ContentFile(SAMPLE_PNG) }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', @@ -142,7 +143,7 @@ def test_patch_organizer_settings_file(token_client, organizer): '/api/v1/upload', data={ 'media_type': 'application/pdf', - 'file': ContentFile('file.pdf', 'invalid pdf content') + 'file': ContentFile('invalid pdf content') }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.pdf"', diff --git a/src/tests/api/test_upload.py b/src/tests/api/test_upload.py index c9b8b765d3..955130f378 100644 --- a/src/tests/api/test_upload.py +++ b/src/tests/api/test_upload.py @@ -19,8 +19,14 @@ # You should have received a copy of the GNU Affero General Public License along with this program. If not, see # . # +import io +import math + import pytest from django.core.files.base import ContentFile +from PIL import Image +from PIL.Image import MAX_IMAGE_PIXELS +from tests.const import SAMPLE_PNG @pytest.mark.django_db @@ -29,7 +35,7 @@ def test_upload_file(token_client): '/api/v1/upload', data={ 'media_type': 'application/pdf', - 'file': ContentFile('file.pdf', 'invalid pdf content') + 'file': ContentFile('invalid pdf content') }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.pdf"', @@ -44,9 +50,74 @@ def test_upload_file_extension_mismatch(token_client): '/api/v1/upload', data={ 'media_type': 'application/pdf', - 'file': ContentFile('file.png', 'invalid pdf content') + 'file': ContentFile('invalid pdf content') }, format='upload', HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', ) assert r.status_code == 400 + assert r.data == ['File name "file.png" has an invalid extension for type "application/pdf"'] + + +@pytest.mark.django_db +def test_upload_file_extension_not_allowed(token_client): + r = token_client.post( + '/api/v1/upload', + data={ + 'media_type': 'application/octet-stream', + 'file': ContentFile('invalid pdf content') + }, + format='upload', + HTTP_CONTENT_DISPOSITION='attachment; filename="file.bin"', + ) + assert r.status_code == 400 + assert r.data == ['Content type "application/octet-stream" is not allowed'] + + +@pytest.mark.django_db +def test_upload_invalid_image(token_client): + r = token_client.post( + '/api/v1/upload', + data={ + 'media_type': 'image/png', + 'file': ContentFile('invalid png content') + }, + format='upload', + HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', + ) + assert r.status_code == 400 + assert r.data == ['Upload a valid image. The file you uploaded was either not an image or a corrupted image.'] + + +@pytest.mark.django_db +def test_upload_valid_image(token_client): + r = token_client.post( + '/api/v1/upload', + data={ + 'media_type': 'image/png', + 'file': ContentFile(SAMPLE_PNG) + }, + format='upload', + HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', + ) + assert r.status_code == 201 + + +@pytest.mark.django_db +@pytest.mark.filterwarnings("ignore") +def test_upload_image_with_invalid_dimensions(token_client): + d = int(math.sqrt(MAX_IMAGE_PIXELS)) + 100 + img = Image.new('RGB', (d, d), color='red') + output = io.BytesIO() + img.save(output, format='PNG') + r = token_client.post( + '/api/v1/upload', + data={ + 'media_type': 'image/png', + 'file': ContentFile(output.getvalue()) + }, + format='upload', + HTTP_CONTENT_DISPOSITION='attachment; filename="file.png"', + ) + assert r.status_code == 400 + assert r.data == ['The file you uploaded has a very large number of pixels, please upload a picture with smaller dimensions.'] diff --git a/src/tests/const.py b/src/tests/const.py new file mode 100644 index 0000000000..b744653fed --- /dev/null +++ b/src/tests/const.py @@ -0,0 +1,26 @@ +# +# This file is part of pretix (Community Edition). +# +# Copyright (C) 2014-2020 Raphael Michel and contributors +# Copyright (C) 2020-2021 rami.io GmbH and contributors +# +# This program is free software: you can redistribute it and/or modify it under the terms of the GNU Affero General +# Public License as published by the Free Software Foundation in version 3 of the License. +# +# ADDITIONAL TERMS APPLY: Pursuant to Section 7 of the GNU Affero General Public License, additional terms are +# applicable granting you additional permissions and placing additional restrictions on your usage of this software. +# Please refer to the pretix LICENSE file to obtain the full terms applicable to this work. If you did not receive +# this file, see . +# +# This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied +# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more +# details. +# +# You should have received a copy of the GNU Affero General Public License along with this program. If not, see +# . +# + +# This is a very short, but valid PNG +SAMPLE_PNG = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x01\x00\x00\x00\x01\x00\x01\x03\x00\x00\x00f\xbc:%\x00\x00' \ + b'\x00\x03PLTE\xb5\xd0\xd0c\x04\x16\xea\x00\x00\x00\x1fIDATh\x81\xed\xc1\x01\r\x00\x00\x00\xc2\xa0\xf7Om' \ + b'\x0e7\xa0\x00\x00\x00\x00\x00\x00\x00\x00\xbe\r!\x00\x00\x01\x9a`\xe1\xd5\x00\x00\x00\x00IEND\xaeB`\x82'