Skip to content

Commit 49b2545

Browse files
authored
Merge pull request #1316 from makeabilitylab/6-validate-filetypes-on-upload
feat(uploads): validate file types on upload (#6)
2 parents 47c43bd + 543f950 commit 49b2545

9 files changed

Lines changed: 407 additions & 10 deletions

File tree

website/models/artifact.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import os # for file handling
44
import website.utils.fileutils as ml_fileutils # for custom file handling
55
from sortedm2m.fields import SortedManyToManyField
6+
from website.utils.upload_validators import validate_pdf_upload, validate_raw_file_upload
67

78
# This retrieves a Python logging instance (or creates it)
89
_logger = logging.getLogger(__name__)
@@ -34,9 +35,9 @@ class Artifact(models.Model):
3435
location.help_text = "The geographic location of where this artifact was presented"
3536

3637
# The artifacts themselves
37-
pdf_file = models.FileField(upload_to=get_upload_dir, null=True, default=None, max_length=255)
38+
pdf_file = models.FileField(upload_to=get_upload_dir, null=True, default=None, max_length=255, validators=[validate_pdf_upload])
3839
pdf_file.help_text = "The rendered PDF of the artifact"
39-
raw_file = models.FileField(upload_to=get_upload_dir, blank=True, null=True, default=None, max_length=255)
40+
raw_file = models.FileField(upload_to=get_upload_dir, blank=True, null=True, default=None, max_length=255, validators=[validate_raw_file_upload])
4041
raw_file.help_text = "The raw file (e.g., pptx, keynote) for the artifact. While not required, this is "\
4142
"<b>highly</b> recommended as it creates a better archive of the work"
4243
thumbnail = models.ImageField(upload_to=get_upload_thumbnail_dir, editable=False, null=True, max_length=255)

website/models/banner.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from django.db.models.signals import pre_delete, post_save, m2m_changed, post_delete
44

55
from website.utils.fileutils import UniquePathAndRename
6+
from website.utils.upload_validators import validate_image_upload, validate_video_upload
67
from image_cropping import ImageRatioField
78

89
from .project import Project
@@ -17,13 +18,13 @@ class Banner(models.Model):
1718
landing_page = models.BooleanField(default=False)
1819
landing_page.help_text = 'Check this box if this banner should appear on the landing page.'
1920

20-
image = models.ImageField(blank=True, upload_to=UniquePathAndRename(UPLOAD_DIR, True), max_length=255)
21+
image = models.ImageField(blank=True, upload_to=UniquePathAndRename(UPLOAD_DIR, True), max_length=255, validators=[validate_image_upload])
2122
cropping = ImageRatioField('image', '1600x500', free_crop=False)
2223
image.help_text = 'After choosing an image, crop it right here using the cropper below — no need to save first.\
2324
Please note that since we are using a responsive design with fixed height banners, your selected image may appear\
2425
differently on various screens.'
2526

26-
video = models.FileField(upload_to=UniquePathAndRename(VIDEO_UPLOAD_DIR, True), blank=True, null=True)
27+
video = models.FileField(upload_to=UniquePathAndRename(VIDEO_UPLOAD_DIR, True), blank=True, null=True, validators=[validate_video_upload])
2728
video.help_text = "Add in a background video. Ideally, video should be 10MB or less. If both a video and image are specified, the video is prioritized. The image is fallback."
2829

2930
alt_text = models.CharField(max_length=1024, blank=True, null=True)

website/models/news.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from ckeditor_uploader.fields import RichTextUploadingField
66
from website.utils.fileutils import UniquePathAndRename
7+
from website.utils.upload_validators import validate_image_upload
78
from image_cropping import ImageRatioField
89

910
from django.utils.text import slugify
@@ -44,7 +45,7 @@ def get_thumbnail_size_as_str():
4445
content = RichTextUploadingField(config_name='default')
4546

4647
# Following the scheme of above thumbnails in other models
47-
image = models.ImageField(blank=True, upload_to=UniquePathAndRename("news", True), max_length=255)
48+
image = models.ImageField(blank=True, upload_to=UniquePathAndRename("news", True), max_length=255, validators=[validate_image_upload])
4849
image.help_text = 'After choosing an image, crop it right here using the cropper below — no need to save first.'
4950

5051
# We use the django-image-cropping ImageRatioField https://github.com/jonasundderwolf/django-image-cropping

website/models/person.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from website.models.project_role import ProjectRole
77
from django.core.files import File
88
import website.utils.fileutils as ml_fileutils
9+
from website.utils.upload_validators import validate_image_upload
910

1011
from django.db.models.functions import Coalesce
1112
from django.conf import settings
@@ -135,7 +136,7 @@ def get_thumbnail_size_as_str():
135136
# Note: the ImageField requires the pillow library
136137
# We use the get_unique_path function because otherwise if two people use the same
137138
# filename (something generic like picture.jpg), one will overwrite the other.
138-
image = models.ImageField(blank=True, upload_to=get_upload_to_for_person, max_length=255)
139+
image = models.ImageField(blank=True, upload_to=get_upload_to_for_person, max_length=255, validators=[validate_image_upload])
139140
image.help_text = 'After choosing an image, crop it right here using the cropper below — no need to save first.'
140141

141142
# We use the django-image-cropping ImageRatioField https://github.com/jonasundderwolf/django-image-cropping
@@ -145,7 +146,7 @@ def get_thumbnail_size_as_str():
145146
cropping = ImageRatioField('image', get_thumbnail_size_as_str(), size_warning=True)
146147

147148
# This is the hover image (aka easter egg)
148-
easter_egg = models.ImageField(blank=True, null=True, upload_to=get_upload_to_for_person_easter_egg, max_length=255)
149+
easter_egg = models.ImageField(blank=True, null=True, upload_to=get_upload_to_for_person_easter_egg, max_length=255, validators=[validate_image_upload])
149150
easter_egg.help_text = mark_safe("You do not have to set this field. It defaults to a Star Wars\
150151
Rebels LEGO character from <a href='https://github.com/makeabilitylab/makeabilitylabwebsite/tree/master/media/images/StarWarsFiguresFullSquare/Rebels'>here</a>\
151152
but you can use whatever you want. This image is shown on mouseover on the people.html page.")

website/models/photo.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from django.db import models
22
from django.utils.safestring import mark_safe
33
from image_cropping import ImageRatioField
4+
from website.utils.upload_validators import validate_image_upload
45

56
from .project import Project
67

@@ -12,7 +13,7 @@ class Photo(models.Model):
1213
def get_cropping_size_as_str():
1314
return f"{DEFAULT_CROPPING_SIZE[0]}x{DEFAULT_CROPPING_SIZE[1]}"
1415

15-
picture = models.ImageField(upload_to='projects/images/', max_length=255)
16+
picture = models.ImageField(upload_to='projects/images/', max_length=255, validators=[validate_image_upload])
1617

1718
# TODO: force both caption and alt_text to be non-null and non-blank
1819
# This requires a migration so need to talk with Matt/Jason in IT about it.

website/models/project.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from django.db.models.functions import Coalesce
55

66
from image_cropping import ImageRatioField
7+
from website.utils.upload_validators import validate_image_upload
78

89
from datetime import date, datetime, timedelta
910
from django.utils import timezone
@@ -82,7 +83,7 @@ def get_thumbnail_size_as_str():
8283

8384
# pis = models.ManyToOneField(Person, blank=True, null=True)
8485
# TODO: consider switching gallery_image var name to thumbnail
85-
gallery_image = models.ImageField(upload_to=IMAGE_DIR, blank=True, null=True, max_length=255)
86+
gallery_image = models.ImageField(upload_to=IMAGE_DIR, blank=True, null=True, max_length=255, validators=[validate_image_upload])
8687
gallery_image.help_text = "This is the image which will show up on the project gallery page.\
8788
It is not displayed anywhere else. After choosing an image, crop it right here\
8889
using the cropper below — no need to save first."

website/models/sponsor.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from django.db import models
22
from image_cropping import ImageRatioField
3+
from website.utils.upload_validators import validate_image_upload
34
import os
45

56
SPONSOR_THUMBNAIL_SIZE = (245, 245)
@@ -18,7 +19,7 @@ def get_thumbnail_size_as_str():
1819
short_name = models.CharField(max_length=255, null=True)
1920
short_name.help_text = "Short name for the sponsor (e.g., NSF)"
2021

21-
icon = models.ImageField(upload_to=ICON_DIR, blank=True, null=True, max_length=255)
22+
icon = models.ImageField(upload_to=ICON_DIR, blank=True, null=True, max_length=255, validators=[validate_image_upload])
2223
icon.help_text = "Icon for the sponsor (e.g., NSF logo)"
2324

2425
alt_text = models.CharField(max_length=1024, blank=True, null=True)
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
"""
2+
Unit tests for website.utils.upload_validators (issue #6).
3+
4+
Pure-logic tests: each validator is a function over an uploaded file, so these
5+
use SimpleTestCase + SimpleUploadedFile with crafted bytes — no DB, runs in ms.
6+
7+
Each category covers four cases:
8+
* accept a file whose extension AND bytes are valid,
9+
* reject a disallowed extension,
10+
* reject a renamed payload (allowed extension, wrong/dangerous bytes),
11+
* plus category-specific cases (HEIC guidance, .fig/.sketch for raw_file).
12+
"""
13+
14+
from django.core.exceptions import ValidationError
15+
from django.core.files.uploadedfile import SimpleUploadedFile
16+
from django.test import SimpleTestCase
17+
18+
from website.utils.upload_validators import (
19+
validate_image_upload,
20+
validate_pdf_upload,
21+
validate_raw_file_upload,
22+
validate_video_upload,
23+
)
24+
25+
26+
# --- Sample file headers ---------------------------------------------------
27+
28+
PNG = b"\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR" + b"\x00" * 16
29+
JPEG = b"\xff\xd8\xff\xe0\x00\x10JFIF\x00\x01" + b"\x00" * 16
30+
GIF = b"GIF89a\x01\x00\x01\x00\x80\x00\x00" + b"\x00" * 16
31+
WEBP = b"RIFF\x24\x00\x00\x00WEBPVP8 " + b"\x00" * 16
32+
PDF = b"%PDF-1.4\n%\xe2\xe3\xcf\xd3\n" + b"\x00" * 16
33+
MP4 = b"\x00\x00\x00\x18ftypmp42\x00\x00\x00\x00" + b"\x00" * 16
34+
WEBM = b"\x1aE\xdf\xa3\x01\x00\x00\x00" + b"\x00" * 16
35+
ZIP = b"PK\x03\x04\x14\x00\x00\x00\x08\x00" + b"\x00" * 16 # pptx/docx/key/sketch/zip
36+
OLE = b"\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1" + b"\x00" * 16 # legacy ppt/doc
37+
FIG = b"fig-kiwi\x0f\x00\x00\x00\x01\x02\x03" + b"\x00" * 16 # proprietary binary
38+
HTML = b"<!DOCTYPE html>\n<html><body>hi</body></html>"
39+
SVG = b'<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>'
40+
41+
42+
def _upload(name, content):
43+
return SimpleUploadedFile(name, content)
44+
45+
46+
class _CommittedFile:
47+
"""
48+
Minimal stand-in for an already-stored FieldFile (``_committed = True``),
49+
i.e. an existing file on an unchanged record. Validators should skip it.
50+
"""
51+
52+
def __init__(self, name):
53+
self.name = name
54+
self._committed = True
55+
56+
def seek(self, *a):
57+
raise AssertionError("a committed file should not be read by validators")
58+
59+
def read(self, *a):
60+
raise AssertionError("a committed file should not be read by validators")
61+
62+
63+
class ImageValidatorTests(SimpleTestCase):
64+
def test_accepts_valid_images(self):
65+
for name, content in [
66+
("a.png", PNG), ("a.jpg", JPEG), ("a.jpeg", JPEG),
67+
("a.gif", GIF), ("a.webp", WEBP),
68+
]:
69+
with self.subTest(name=name):
70+
validate_image_upload(_upload(name, content)) # no raise
71+
72+
def test_rejects_disallowed_extension(self):
73+
with self.assertRaises(ValidationError):
74+
validate_image_upload(_upload("a.svg", SVG))
75+
with self.assertRaises(ValidationError):
76+
validate_image_upload(_upload("a.html", HTML))
77+
78+
def test_rejects_renamed_payload(self):
79+
# Allowed extension, but the bytes are HTML, not an image.
80+
with self.assertRaises(ValidationError) as ctx:
81+
validate_image_upload(_upload("evil.png", HTML))
82+
self.assertEqual(ctx.exception.code, "invalid_image_content")
83+
84+
def test_heic_gets_guiding_message(self):
85+
for name in ("photo.heic", "photo.HEIC", "photo.heif"):
86+
with self.subTest(name=name):
87+
with self.assertRaises(ValidationError) as ctx:
88+
validate_image_upload(_upload(name, PNG))
89+
self.assertEqual(ctx.exception.code, "heic_not_supported")
90+
91+
92+
class PdfValidatorTests(SimpleTestCase):
93+
def test_accepts_valid_pdf(self):
94+
validate_pdf_upload(_upload("paper.pdf", PDF))
95+
96+
def test_rejects_disallowed_extension(self):
97+
with self.assertRaises(ValidationError):
98+
validate_pdf_upload(_upload("paper.exe", PDF))
99+
100+
def test_rejects_renamed_payload(self):
101+
with self.assertRaises(ValidationError) as ctx:
102+
validate_pdf_upload(_upload("evil.pdf", HTML))
103+
self.assertEqual(ctx.exception.code, "invalid_pdf_content")
104+
105+
106+
class VideoValidatorTests(SimpleTestCase):
107+
def test_accepts_valid_videos(self):
108+
validate_video_upload(_upload("clip.mp4", MP4))
109+
validate_video_upload(_upload("clip.mov", MP4))
110+
validate_video_upload(_upload("clip.webm", WEBM))
111+
112+
def test_rejects_disallowed_extension(self):
113+
with self.assertRaises(ValidationError):
114+
validate_video_upload(_upload("clip.avi", MP4))
115+
116+
def test_rejects_renamed_payload(self):
117+
with self.assertRaises(ValidationError) as ctx:
118+
validate_video_upload(_upload("evil.mp4", HTML))
119+
self.assertEqual(ctx.exception.code, "invalid_video_content")
120+
121+
122+
class RawFileValidatorTests(SimpleTestCase):
123+
def test_accepts_known_source_formats(self):
124+
for name, content in [
125+
("talk.pptx", ZIP), ("talk.key", ZIP), ("doc.docx", ZIP),
126+
("src.zip", ZIP), ("legacy.ppt", OLE), ("paper.pdf", PDF),
127+
]:
128+
with self.subTest(name=name):
129+
validate_raw_file_upload(_upload(name, content)) # no raise
130+
131+
def test_accepts_proprietary_design_files(self):
132+
# .fig / .sketch are accepted by extension; the denylist content check
133+
# passes any non-web-executable bytes, so we don't need their signatures.
134+
validate_raw_file_upload(_upload("poster.fig", FIG))
135+
validate_raw_file_upload(_upload("poster.sketch", ZIP))
136+
137+
def test_rejects_disallowed_extension(self):
138+
with self.assertRaises(ValidationError):
139+
validate_raw_file_upload(_upload("page.html", HTML))
140+
with self.assertRaises(ValidationError):
141+
validate_raw_file_upload(_upload("image.svg", SVG))
142+
143+
def test_rejects_web_executable_content_via_rename(self):
144+
# Allowed extension (.fig) but HTML bytes -> caught by the denylist.
145+
with self.assertRaises(ValidationError) as ctx:
146+
validate_raw_file_upload(_upload("evil.fig", HTML))
147+
self.assertEqual(ctx.exception.code, "invalid_raw_content")
148+
149+
150+
class ExistingFileGateTests(SimpleTestCase):
151+
"""An already-stored file (unchanged record edit) is skipped, even if its
152+
extension/content would fail today's rules. Re-validating it adds no
153+
security and would break editing legacy records."""
154+
155+
def test_committed_files_are_not_validated(self):
156+
# Each of these would fail if validated as a new upload; the gate
157+
# short-circuits before the extension/content checks (and before any
158+
# read of the file, which _CommittedFile asserts against).
159+
validate_image_upload(_CommittedFile("legacy.bmp"))
160+
validate_pdf_upload(_CommittedFile("legacy.txt"))
161+
validate_video_upload(_CommittedFile("legacy.avi"))
162+
validate_raw_file_upload(_CommittedFile("legacy.tex"))

0 commit comments

Comments
 (0)