Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-h85r-xv4w-cg8g
Fix GHSA-h85r-xv4w-cg8g - perform backend file type validation
  • Loading branch information
sergei-maertens committed Jun 13, 2022
2 parents 3e8c9cc + 634254e commit 0978a29
Show file tree
Hide file tree
Showing 12 changed files with 228 additions and 13 deletions.
1 change: 1 addition & 0 deletions Dockerfile
Expand Up @@ -74,6 +74,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
vim \
mime-support \
postgresql-client \
libmagic1 \
libxmlsec1 \
libxmlsec1-openssl \
gettext \
Expand Down
3 changes: 2 additions & 1 deletion INSTALL.rst
Expand Up @@ -17,13 +17,14 @@ You need the following libraries and/or programs:
* `Python`_ 3.8 or above
* Python `Virtualenv`_ and `Pip`_
* `PostgreSQL`_ 10 or above
* `Node.js`_ (LTS version, see the Dockerfile for version information)
* `Node.js`_ (LTS version, see ``.nvmrc`` for version information)
* `npm`_
* `yarn`_

You will also need the following libraries:

* pkg-config
* libmagic1
* libxml2-dev
* libxmlsec1-dev
* libxmlsec1-openssl
Expand Down
3 changes: 2 additions & 1 deletion requirements/base.in
Expand Up @@ -10,11 +10,12 @@ lxml
O365 # microsoft graph
phonenumbers
Pillow # handle images
portalocker[redis]
psycopg2 # database driver
pytz # handle timezones
python-dotenv # environment variables for secrets
python-decouple # processing of envvar configs
portalocker[redis]
python-magic
tablib[xlsx]
xmltodict
self-certifi
Expand Down
2 changes: 2 additions & 0 deletions requirements/base.txt
Expand Up @@ -316,6 +316,8 @@ python-decouple==3.3
# via -r requirements/base.in
python-dotenv==0.14.0
# via -r requirements/base.in
python-magic==0.4.27
# via -r requirements/base.in
python3-saml @ git+https://github.com/maykinmedia/python3-saml.git@f587f77b78be79d51139f29a957b406072e2b537
# via -r requirements/base.in
pytz==2021.3
Expand Down
4 changes: 4 additions & 0 deletions requirements/ci.txt
Expand Up @@ -622,6 +622,10 @@ python-dotenv==0.14.0
# via
# -c requirements/base.txt
# -r requirements/base.txt
python-magic==0.4.27
# via
# -c requirements/base.txt
# -r requirements/base.txt
python3-saml @ git+https://github.com/maykinmedia/python3-saml.git@f587f77b78be79d51139f29a957b406072e2b537
# via
# -c requirements/base.txt
Expand Down
4 changes: 4 additions & 0 deletions requirements/dev.txt
Expand Up @@ -737,6 +737,10 @@ python-dotenv==0.14.0
# via
# -c requirements/ci.txt
# -r requirements/ci.txt
python-magic==0.4.27
# via
# -c requirements/ci.txt
# -r requirements/ci.txt
python3-saml @ git+https://github.com/maykinmedia/python3-saml.git@f587f77b78be79d51139f29a957b406072e2b537
# via
# -c requirements/ci.txt
Expand Down
30 changes: 30 additions & 0 deletions src/openforms/submissions/attachments.py
@@ -1,5 +1,6 @@
import os.path
import re
from collections import defaultdict
from datetime import timedelta
from typing import Iterable, Optional, Tuple
from urllib.parse import urlparse
Expand All @@ -9,9 +10,11 @@
from django.urls import Resolver404, resolve
from django.utils.translation import gettext as _

import magic
import PIL
from glom import glom
from PIL import Image
from rest_framework.exceptions import ValidationError

from openforms.api.exceptions import RequestEntityTooLarge
from openforms.conf.utils import Filesize
Expand Down Expand Up @@ -85,6 +88,7 @@ def attach_uploads_to_submission_step(submission_step: SubmissionStep) -> list:
uploads = resolve_uploads_from_data(components, submission_step.data)

result = list()
validation_errors = defaultdict(list)
for key, (component, uploads) in uploads.items():
# grab resize settings
resize_apply = glom(component, "of.image.resize.apply", default=False)
Expand All @@ -94,6 +98,7 @@ def attach_uploads_to_submission_step(submission_step: SubmissionStep) -> list:
component, "of.image.resize.height", default=DEFAULT_IMAGE_MAX_SIZE[1]
),
)
allowed_mime_types = glom(component, "file.type", default=[])
file_max_size = file_size_cast(
glom(component, "fileMaxSize", default="") or settings.MAX_FILE_UPLOAD_SIZE
)
Expand All @@ -112,6 +117,28 @@ def attach_uploads_to_submission_step(submission_step: SubmissionStep) -> list:
),
)

# perform content type validation
with upload.content.open("rb") as infile:
# 2048 bytes per recommendation of python-magic
file_mime_type = magic.from_buffer(infile.read(2048), mime=True)

invalid_file_type_error = ValidationError(
_("The file '{filename}' is not a valid file type.").format(
filename=upload.file_name
),
code="invalid",
)

if upload.content_type != file_mime_type:
validation_errors[key].append(invalid_file_type_error)
continue

# if no allowed_mime_types are defined on the file component, then all filetypes
# are allowed and we skip validation.
if allowed_mime_types and file_mime_type not in allowed_mime_types:
validation_errors[key].append(invalid_file_type_error)
continue

file_name = append_file_num_postfix(
upload.file_name, base_name, i, len(uploads)
)
Expand All @@ -126,6 +153,9 @@ def attach_uploads_to_submission_step(submission_step: SubmissionStep) -> list:
# see https://github.com/open-formulieren/open-forms/issues/507
resize_submission_attachment.delay(attachment.id, resize_size)

if validation_errors:
raise ValidationError(validation_errors)

return result


Expand Down
11 changes: 5 additions & 6 deletions src/openforms/submissions/models/submission_files.py
Expand Up @@ -95,19 +95,18 @@ def create_from_upload(
False,
)
except self.model.DoesNotExist:
return (
self.create(
with upload.content.open("rb") as content:
instance = self.create(
submission_step=submission_step,
temporary_file=upload,
form_key=form_key,
# wrap in File() so it will be physically copied
content=File(upload.content, name=upload.file_name),
content=File(content, name=upload.file_name),
content_type=upload.content_type,
original_name=upload.file_name,
file_name=file_name,
),
True,
)
)
return (instance, True)


class SubmissionFileAttachment(DeleteFileFieldFilesMixin, models.Model):
Expand Down
7 changes: 6 additions & 1 deletion src/openforms/submissions/tests/factories.py
Expand Up @@ -5,6 +5,7 @@
from django.utils import timezone

import factory
import magic
from glom import PathAccessError, glom

from openforms.forms.models import FormVariable
Expand Down Expand Up @@ -188,12 +189,16 @@ class Meta:

class TemporaryFileUploadFactory(factory.django.DjangoModelFactory):
file_name = factory.Faker("file_name")
content_type = factory.Faker("mime_type")
content = factory.django.FileField(filename="file.dat", data=b"content")

class Meta:
model = TemporaryFileUpload

@factory.lazy_attribute
def content_type(self) -> str:
buffer = self.content.read(2048)
return magic.from_buffer(buffer, mime=True)


class SubmissionFileAttachmentFactory(factory.django.DjangoModelFactory):
submission_step = factory.SubFactory(SubmissionStepFactory)
Expand Down
5 changes: 5 additions & 0 deletions src/openforms/submissions/tests/files/README.md
@@ -0,0 +1,5 @@
# Test files

- `image-256x256.png` is a 256px red square.
- `image-256x256.pdf` is the same file as `image-256x256.png` but pretends to be a PDF by extension.
This file is used for content-type validation.
Binary file not shown.

0 comments on commit 0978a29

Please sign in to comment.