DefectDojo · devGregA · Jul 20, 2022 · Jul 14, 2022 · Jul 14, 2022 · Jul 15, 2022
diff --git a/dojo/forms.py b/dojo/forms.py
@@ -658,14 +658,23 @@ def __init__(self, *args, **kwargs):
         self.fields['accepted_findings'].queryset = get_authorized_findings(Permissions.Risk_Acceptance)
 
 
-class UploadFileForm(forms.ModelForm):
-
-    class Meta:
-        model = FileUpload
-        fields = ['title', 'file']
+class BaseManageFileFormSet(forms.BaseModelFormSet):
+    def clean(self):
+        """Validate the IP/Mask combo is in CIDR format"""
+        if any(self.errors):
+            # Don't bother validating the formset unless each form is valid on its own
+            return
+        for form in self.forms:
+            print(dir(form))
+            file = form.cleaned_data.get('file', None)
+            if file:
+                ext = os.path.splitext(file.name)[1]  # [0] returns path+filename
+                valid_extensions = settings.FILE_UPLOAD_TYPES
+                if ext.lower() not in valid_extensions:
+                    form.add_error('file', 'Unsupported file extension.')
 
 
-ManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=['title', 'file'], can_delete=True)
+ManageFileFormSet = modelformset_factory(FileUpload, extra=3, max_num=10, fields=['title', 'file'], can_delete=True, formset=BaseManageFileFormSet)
 
 
 class ReplaceRiskAcceptanceProofForm(forms.ModelForm):

diff --git a/dojo/settings/settings.dist.py b/dojo/settings/settings.dist.py
@@ -231,6 +231,9 @@
     DD_DELETE_PREVIEW=(bool, True),
     # Feature toggle for new authorization for configurations
     DD_FEATURE_CONFIGURATION_AUTHORIZATION=(bool, True),
+    # List of acceptable file types that can be uploaded to a given object via arbitrary file upload
+    DD_FILE_UPLOAD_TYPES=(list, ['.txt', '.pdf', '.json', '.xml', '.csv', '.yml', '.png', '.jpeg',
+                                 '.html', '.sarif', '.xslx', '.doc', '.html', '.js', '.nessus', '.zip']),
 )
 
 
@@ -1471,3 +1474,5 @@ def saml2_attrib_map_format(dict):
     'SNYK': 'https://snyk.io/vuln/',
     'RUSTSEC': 'https://rustsec.org/advisories/',
 }
+# List of acceptable file types that can be uploaded to a given object via arbitrary file upload
+FILE_UPLOAD_TYPES = env("DD_FILE_UPLOAD_TYPES")
diff --git a/dojo/templates/dojo/view_eng.html b/dojo/templates/dojo/view_eng.html
@@ -677,12 +677,13 @@ <h4>Files<span class="pull-right">
                             {% for file in files %}
                                 <div class="col-md-2" style="text-align: center">
                                     <div class="row">
-                                        <a href="{{ request.scheme }}://{{ request.get_host }}{{ file.file.url }}" target="_blank">
-                                        {% if file.file.url|get_thumbnail %}
-                                            <img src="{{ file.file.url }}" alt="thumbnail" style="width:150px">
-                                        {% else %}
-                                            <span style="font-size: 50px;" class="glyphicon glyphicon-file"></span>
-                                        {% endif %}
+                                        {% url 'access_file' fid=file.id oid=eng.id obj_type='Engagement' as image_url %}
+                                        <a href="{{ image_url }}" target="_blank">
+                                            {% if file|get_thumbnail %}
+                                                <img src="{{ image_url }}" alt="thumbnail" style="width:150px">
+                                            {% else %}
+                                                <span style="font-size: 50px;" class="glyphicon glyphicon-file"></span>
+                                            {% endif %}
                                         </a>
                                     </div>
                                     <div class="row">

diff --git a/dojo/templates/dojo/view_finding.html b/dojo/templates/dojo/view_finding.html
@@ -763,9 +763,10 @@ <h4>Files<span class="pull-right">
                             {% for file in files %}
                             <div class="col-md-2" style="text-align: center">
                                 <div class="row">
-                                    <a href="{{ request.scheme }}://{{ request.get_host }}{{ file.file.url }}" target="_blank">
-                                        {% if file.file.url|get_thumbnail %}
-                                            <img src="{{ file.file.url }}" alt="thumbnail" style="width:150px">
+                                    {% url 'access_file' fid=file.id oid=finding.id obj_type='Finding' as image_url %}
+                                    <a href="{{ image_url }}" target="_blank">
+                                        {% if file|get_thumbnail %}
+                                            <img src="{{ image_url }}" alt="thumbnail" style="width:150px">
                                         {% else %}
                                             <span style="font-size: 50px;" class="glyphicon glyphicon-file"></span>
                                         {% endif %}

diff --git a/dojo/templates/dojo/view_test.html b/dojo/templates/dojo/view_test.html
@@ -1136,9 +1136,10 @@ <h4>Files<span class="pull-right">&nbsp;
                 {% for file in files %}
                 <div class="col-md-2" style="text-align: center">
                     <div class="row">
-                        <a href="{{ request.scheme }}://{{ request.get_host }}{{ file.file.url }}" target="_blank">
-                            {% if file.file.url|get_thumbnail %}
-                                <img src="{{ file.file.url }}" alt="thumbnail" style="width:150px">
+                        {% url 'access_file' fid=file.id oid=test.id obj_type='Test' as image_url %}
+                        <a href="{{ image_url }}" target="_blank">
+                            {% if file|get_thumbnail %}
+                                <img src="{{ image_url }}" alt="thumbnail" style="width:150px">
                             {% else %}
                                 <span style="font-size: 50px;" class="glyphicon glyphicon-file"></span>
                             {% endif %}

diff --git a/dojo/templatetags/display_tags.py b/dojo/templatetags/display_tags.py
@@ -838,9 +838,9 @@ def jira_change(obj):
 
 
 @register.filter
-def get_thumbnail(filename):
+def get_thumbnail(file):
     from pathlib import Path
-    file_format = Path(filename).suffix[1:]
+    file_format = Path(file.file.url).suffix[1:]
     return file_format in supported_file_formats
 
 

diff --git a/dojo/urls.py b/dojo/urls.py
@@ -1,6 +1,5 @@
 from django.conf import settings
 from django.conf.urls import include, url
-from django.conf.urls.static import static
 from django.contrib import admin
 from rest_framework.routers import DefaultRouter
 from rest_framework.authtoken import views as tokenviews
@@ -181,7 +180,8 @@
 
     url(r'^robots.txt', lambda x: HttpResponse("User-Agent: *\nDisallow: /", content_type="text/plain"), name="robots_file"),
     url(r'^manage_files/(?P<oid>\d+)/(?P<obj_type>\w+)$', views.manage_files, name='manage_files'),
-
+    url(r'^access_file/(?P<fid>\d+)/(?P<oid>\d+)/(?P<obj_type>\w+)$', views.access_file, name='access_file'),
+    url(r'^%s/(?P<path>.*)$' % settings.MEDIA_URL.strip('/'), views.protected_serve, {'document_root': settings.MEDIA_ROOT})
 ]
 
 urlpatterns += survey_urls
@@ -200,9 +200,6 @@
         #  django admin
         urlpatterns += [url(r'^%sadmin/' % get_system_setting('url_prefix'), admin.site.urls)]
 
-if settings.DEBUG:
-    urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)
-
 # sometimes urlpatterns needed be added from local_settings.py to avoid having to modify core defect dojo files
 if hasattr(settings, 'EXTRA_URL_PATTERNS'):
     urlpatterns += settings.EXTRA_URL_PATTERNS
diff --git a/dojo/views.py b/dojo/views.py
@@ -4,9 +4,11 @@
 from django.contrib.contenttypes.models import ContentType
 from django.contrib import messages
 from django.core.exceptions import PermissionDenied, ObjectDoesNotExist
-from django.http import Http404, HttpResponseRedirect
+from django.http import Http404, HttpResponseRedirect, FileResponse
 from django.conf import settings
 from django.urls import reverse
+from django.contrib.auth.decorators import login_required
+from django.views.static import serve
 from django.shortcuts import render, get_object_or_404
 from dojo.models import Engagement, Test, Finding, Endpoint, Product, FileUpload
 from dojo.filters import LogEntryFilter
@@ -172,3 +174,42 @@ def manage_files(request, oid, obj_type):
             'obj': obj,
             'obj_type': obj_type,
         })
+
+
+# Serve the file only after verifying the user is supposed to see the file
+@login_required
+def protected_serve(request, path, document_root=None, show_indexes=False):
+    file = FileUpload.objects.get(file=path)
+    if not file:
+        raise Http404()
+    object_set = list(file.engagement_set.all()) + list(file.test_set.all()) + list(file.finding_set.all())
+    # Should only one item (but not sure what type) in the list, so O(n=1)
+    for obj in object_set:
+        if isinstance(obj, Engagement):
+            user_has_permission_or_403(request.user, obj, Permissions.Engagement_View)
+        elif isinstance(obj, Test):
+            user_has_permission_or_403(request.user, obj, Permissions.Test_View)
+        elif isinstance(obj, Finding):
+            user_has_permission_or_403(request.user, obj, Permissions.Finding_View)
+    return serve(request, path, document_root, show_indexes)
+
+
+def access_file(request, fid, oid, obj_type, url=False):
+    if obj_type == 'Engagement':
+        obj = get_object_or_404(Engagement, pk=oid)
+        user_has_permission_or_403(request.user, obj, Permissions.Engagement_View)
+    elif obj_type == 'Test':
+        obj = get_object_or_404(Test, pk=oid)
+        user_has_permission_or_403(request.user, obj, Permissions.Test_View)
+    elif obj_type == 'Finding':
+        obj = get_object_or_404(Finding, pk=oid)
+        user_has_permission_or_403(request.user, obj, Permissions.Finding_View)
+    else:
+        raise Http404()
+    # If reaching this far, user must have permission to get file
+    file = get_object_or_404(FileUpload, pk=fid)
+    redirect_url = '{media_root}/{file_name}'.format(
+        media_root=settings.MEDIA_ROOT,
+        file_name=file.file.url.lstrip(settings.MEDIA_URL))
+    print(redirect_url)
+    return FileResponse(open(redirect_url))
diff --git a/nginx/nginx.conf b/nginx/nginx.conf
@@ -30,10 +30,6 @@ http {
     location /static/ {
       alias /usr/share/nginx/html/static/;
     }
-    location /media/ {
-        add_header Content-Disposition attachment;
-        alias /usr/share/nginx/html/media/;
-    }
     location / {
       include /run/defectdojo/uwsgi_pass;
       include /etc/nginx/wsgi_params;

diff --git a/nginx/nginx_TLS.conf b/nginx/nginx_TLS.conf
@@ -47,10 +47,6 @@ http {
     location /static/ {
       alias /usr/share/nginx/html/static/;
     }
-    location /media/ {
-      add_header Content-Disposition attachment;
-      alias /usr/share/nginx/html/media/;
-    }
     location / {
       include /run/defectdojo/uwsgi_pass;
       include /etc/nginx/wsgi_params;