Merge pull request #209 from BenediktMKuehne/reporter-fix

reporter fixes

dev-tools/check_project.sh

@@ -184,7 +184,7 @@ banditer() {
     exit 1
   fi
 
-  mapfile -t PY_SCRIPTS < <(find . -type d -name migrations -prune -false -o -iname "*.py" -not -path "./.venv/*" -not -path "./emba/*")
+  mapfile -t PY_SCRIPTS < <(find . -type d -name migrations -prune -false -o -iname "*.py" -not -path "./.venv/*" -not -path "./emba/*" -not -path "./emba_logs/*")
 
   for PY_SCRIPT in "${PY_SCRIPTS[@]}"; do
     echo -e "\\n""${GREEN}""Run bandit on ${PY_SCRIPT}:""${NC}""\\n"
@@ -294,11 +294,11 @@ list_linter_exceptions(){
       SEARCH_TYPE_="sh"
       ;;
     bandit)
-      SEARCH_PAR_="nosec"
+      SEARCH_PAR_=" nosec"
       SEARCH_TYPE_="py"
       ;;
     pylint)
-      SEARCH_PAR_="pylint"
+      SEARCH_PAR_="pylint: disable"
       SEARCH_TYPE_="py"
       ;;
     djlint)
@@ -350,13 +350,14 @@ shellchecker
 list_linter_exceptions "shellcheck" "$PWD"
 dockerchecker
 jscheck
-list_linter_exceptions "jshint" "$PWD"
+list_linter_exceptions "jshint" "$PWD/embark"
 templatechecker
-list_linter_exceptions "djlint" "$PWD"
+list_linter_exceptions "djlint" "$PWD/embark"
 pycodestyle_check
 banditer
-list_linter_exceptions "bandit" "$PWD" "${PWD}/.venv"
+list_linter_exceptions "bandit" "$PWD/embark"
 pylinter
+list_linter_exceptions "pylint" "$PWD/embark"
 check_django
 yamlchecker
 copy_right_check 2024 "${PWD}" "${PWD}/emba_logs"

embark/reporter/urls.py

@@ -10,14 +10,12 @@
 
 # view routing
 urlpatterns = [
-    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/emba_logs/html-report/style/<str:img_file>', views.html_report_resource, name='embark-html-report-resource'),
-    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/emba_logs/html-report/<str:html_file>', views.html_report, name='embark-html-report'),
-    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/emba_logs/html-report/<str:html_path>/<str:html_file>', views.html_report_path, name='embark-html-report-path'),
-
+    # TODO get rid of the emba log paths
+    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/<str:html_file>', views.html_report, name='embark-html-report-index'),
+    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/style/<str:img_file>', views.html_report_resource, name='embark-html-report-resource'),
+    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/<path:html_path>/<str:file>', views.html_report_path, name='embark-html-report-path'),
     path('get_load/', views.get_load, name='embark-get-load'),
     path('get_individual_report/<uuid:analysis_id>/', views.get_individual_report, name='embark-get-individual-report'),
     path('get_accumulated_reports/', views.get_accumulated_reports, name='embark-get-accumulated-reports'),
-
-    path(settings.EMBA_LOG_URL + '<uuid:analysis_id>/emba_logs/html-report/<path:html_path>/<str:download_file>/', views.html_report_download, name='embark-html-report-download'),
     path('download_zipped/<uuid:analysis_id>/', views.download_zipped, name='embark-download'),
 ]

embark/reporter/views.py

@@ -1,4 +1,4 @@
-# pylint: disable=W0613,C0206
+# pylint: disable=C0206
 __copyright__ = 'Copyright 2021-2024 Siemens Energy AG'
 __author__ = 'Benedikt Kuehne'
 __license__ = 'MIT'
@@ -11,13 +11,13 @@
 
 from operator import itemgetter
 from http import HTTPStatus
+import re
 from shutil import move
 import codecs
 from uuid import UUID
 
 from django.conf import settings
 from django.forms import model_to_dict
-from django.http.response import Http404
 from django.shortcuts import redirect, render
 from django.contrib import messages
 from django.template.loader import get_template
@@ -48,8 +48,15 @@ def reports(request):
 @require_http_methods(["GET"])
 @login_required(login_url='/' + settings.LOGIN_URL)
 def html_report(request, analysis_id, html_file):
-    report_path = Path(f'{settings.EMBA_LOG_ROOT}{request.path[10:]}')
-    if FirmwareAnalysis.objects.filter(id=analysis_id).exists():
+    """
+    Let's the user request any html in the html-report
+    Checks: valid filename
+    TODO test traversal
+    """
+    # make sure the html file is valid
+    html_file_pattern = re.compile(r'^[\w,\s-]+\.html$')
+    report_path = Path(f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/{html_file}')
+    if FirmwareAnalysis.objects.filter(id=analysis_id).exists() and bool(re.match(html_file_pattern, html_file)):
         analysis = FirmwareAnalysis.objects.get(id=analysis_id)
         if analysis.hidden is False or analysis.user == request.user or request.user.is_superuser:
             html_body = get_template(report_path)
@@ -62,87 +69,97 @@ def html_report(request, analysis_id, html_file):
 
 @require_http_methods(["GET"])
 @login_required(login_url='/' + settings.LOGIN_URL)
-def html_report_path(request, analysis_id, html_path, html_file):
-    if FirmwareAnalysis.objects.filter(id=analysis_id).exists():
+def html_report_path(request, analysis_id, html_path, file):
+    """
+    The functions needs to either serve html files or provide download of files in the subdirs
+    Checks: valid filename, path.resolved in correct parent
+    """
+    # make sure the html file is valid
+    file_pattern = re.compile(r'^[\w\.-]+\.(tar.gz|html)$')
+    if FirmwareAnalysis.objects.filter(id=analysis_id).exists() and bool(re.match(file_pattern, file)):
         analysis = FirmwareAnalysis.objects.get(id=analysis_id)
         if analysis.hidden is False or analysis.user == request.user or request.user.is_superuser:
-            report_path = f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/{html_path}/{html_file}'
-            logger.debug("html_report - analysis_id: %s path: %s html_file: %s", analysis_id, html_path, html_file)
-            try:
-                return render(request, report_path, {'embarkBackUrl': reverse('embark-ReportDashboard')}, content_type='text/html')
-            except UnicodeDecodeError as decode_error:
-                logger.error("{%s} with error: %s", report_path, decode_error)
-                # removes all non utf8 chars from html USING: https://stackoverflow.com/questions/191359/how-to-convert-a-file-to-utf-8-in-python
-                # CodeQL issue is not relevant
-                with codecs.open(report_path, "r", encoding='latin1') as source_file:
-                    with codecs.open(f'{report_path}.new', "w", "utf-8") as target_file:
-                        while True:
-                            contents = source_file.read(BLOCKSIZE)
-                            if not contents:
-                                break
-                            target_file.write(contents)
-                # exchange files
-                move(report_path, f'{report_path}.old')
-                move(f'{report_path}.new', report_path)
-                logger.debug("Removed problematic char from %s", report_path)
-                return render(request, report_path, {'embarkBackUrl': reverse('embark-ReportDashboard')}, content_type='text/html')
+            resource_path = f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/{html_path}/{file}'
+            parent_path = os.path.abspath(f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/')
+            if os.path.commonpath([parent_path, resource_path]) == parent_path:
+                if file.endswith(".tar.gz"):
+                    content_type = "text/plain"
+                    try:
+                        with open(resource_path, 'rb') as requested_file:
+                            response = HttpResponse(requested_file.read(), content_type=content_type)
+                            response['Content-Disposition'] = 'attachment; filename=' + requested_file
+                            logger.info("html_report - analysis_id: %s html_path: %s download_file: %s", analysis_id, html_path, requested_file)
+                            return response
+                    except FileNotFoundError:
+                        messages.error(request, "File not found on the server")
+                        logger.error("Couldn't find %s", resource_path)
+                        return redirect("..")
+
+                elif file.endswith(".html"):
+                    content_type = "text/html"
+                    logger.debug("html_report - analysis_id: %s path: %s html_file: %s", analysis_id, html_path, file)
+                    try:
+                        return render(request, resource_path, {'embarkBackUrl': reverse('embark-ReportDashboard')}, content_type=content_type)
+                    except UnicodeDecodeError as decode_error:
+                        logger.error("{%s} with error: %s", resource_path, decode_error)
+                        # removes all non utf8 chars from html USING: https://stackoverflow.com/questions/191359/how-to-convert-a-file-to-utf-8-in-python
+                        # CodeQL issue is not relevant
+                        with codecs.open(resource_path, "r", encoding='latin1') as source_file:
+                            with codecs.open(f'{resource_path}.new', "w", "utf-8") as target_file:
+                                while True:
+                                    contents = source_file.read(BLOCKSIZE)
+                                    if not contents:
+                                        break
+                                    target_file.write(contents)
+                        # exchange files
+                        move(resource_path, f'{resource_path}.old')
+                        move(f'{resource_path}.new', resource_path)
+                        logger.debug("Removed problematic char from %s", resource_path)
+                        return render(request, resource_path, {'embarkBackUrl': reverse('embark-ReportDashboard')}, content_type=content_type)
+                messages.error(request, "Can't server that file")
+                logger.error("Server can't handle that file - %s", request)
+                return redirect("..")
         messages.error(request, "User not authorized")
+        logger.error("User not authorized - %s", request)
+        return redirect("..")
     logger.error("could  not get path - %s", request)
     return redirect("..")
 
 
-@require_http_methods(["GET"])
-@login_required(login_url='/' + settings.LOGIN_URL)
-def html_report_download(request, analysis_id, html_path, download_file):
-    response = Http404("Resource not found")
-    if FirmwareAnalysis.objects.filter(id=analysis_id).exists():
-        analysis = FirmwareAnalysis.objects.get(id=analysis_id)
-        if analysis.hidden is False or analysis.user == request.user or request.user.is_superuser:
-            resource_path = os.path.abspath(f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/{html_path}/{download_file}')
-            parent_path = os.path.abspath(f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/')
-            if os.path.commonpath([parent_path, resource_path]) == parent_path:
-                try:
-                    with open(resource_path, 'rb') as requested_file:
-                        response = HttpResponse(requested_file.read(), content_type="text/plain")
-                        response['Content-Disposition'] = 'attachment; filename=' + download_file
-                        logger.info("html_report - analysis_id: %s html_path: %s download_file: %s", analysis_id, html_path,
-                                    download_file)
-                except FileNotFoundError:
-                    messages.error(request, "File not found on the server")
-                    logger.error("Couldn't find %s", resource_path)
-                    response = HttpResponse("Couldn't find %s", resource_path)
-    return response
-
-
 @require_http_methods(["GET"])
 @login_required(login_url='/' + settings.LOGIN_URL)
 def html_report_resource(request, analysis_id, img_file):
-    if FirmwareAnalysis.objects.filter(id=analysis_id).exists():
-        analysis = FirmwareAnalysis.objects.get(id=analysis_id)
-        if analysis.hidden is False or analysis.user == request.user or request.user.is_superuser:
-            content_type = "text/plain"
-
-            if img_file.endswith(".css"):
-                content_type = "text/css"
-            elif img_file.endswith(".svg"):
-                content_type = "image/svg+xml"
-            elif img_file.endswith(".png"):
-                content_type = "image/png"
-
-            resource_path = Path(f'{settings.EMBA_LOG_ROOT}{request.path[10:]}')
-            logger.info("html_report_resource - analysis_id: %s request.path: %s", analysis_id, request.path)
+    """
+    Serves all resource files needed by the report
+    Chcks: filename validity
+    """
+    # make sure the html file is valid
+    img_file_pattern = re.compile(r'^[\w,\s-]+\.+(css|svg|png)$')
+    if bool(re.match(img_file_pattern, img_file)):
+        if FirmwareAnalysis.objects.filter(id=analysis_id).exists():
+            analysis = FirmwareAnalysis.objects.get(id=analysis_id)
+            if analysis.hidden is False or analysis.user == request.user or request.user.is_superuser:
+                content_type = "text/plain"
+
+                if img_file.endswith(".css"):
+                    content_type = "text/css"
+                elif img_file.endswith(".svg"):
+                    content_type = "image/svg+xml"
+                elif img_file.endswith(".png"):
+                    content_type = "image/png"
+
+                resource_path = Path(f'{settings.EMBA_LOG_ROOT}/{analysis_id}/emba_logs/html-report/style/{img_file}')
+                logger.info("html_report_resource - analysis_id: %s request.path: %s", analysis_id, request.path)
 
-            try:
-                # CodeQL issue is not relevant as the urls are defined via urls.py
-                with open(resource_path, "rb") as file_:
-                    return HttpResponse(file_.read(), content_type=content_type)
-            except IOError as error:
-                logger.error(error)
-                logger.error(request.path)
-    # just in case -> back to report intro
-    report_path = Path(f'{settings.EMBA_LOG_ROOT}{request.path[10:]}')
-    html_body = get_template(report_path)
-    return HttpResponse(html_body.render())
+                try:
+                    # CodeQL issue is not relevant as the urls are defined via urls.py
+                    with open(resource_path, "rb") as file_:
+                        return HttpResponse(file_.read(), content_type=content_type)
+                except IOError as error:
+                    logger.error(error)
+                    logger.error(request.path)
+    logger.error("could  not get path - %s", request)
+    return redirect("..")
 
 
 @require_http_methods(["GET"])

embark/templates/dashboard/individualReportDashboard.html

@@ -36,12 +36,9 @@ <h1><center>Firmware security scanning details</center></h1>
             <table class="table table-striped table-borderless table-individualrep">
                 <div class="buttonRow d-flex">
                     <!-- Quick and dirty JS solution. Need to fix the report id for using django mechanisms -->
-                    <form action={% url 'embark-html-report' analysis_id 'index.html' %} method='get'>
+                    <form action={% url 'embark-html-report-index' analysis_id 'index.html' %} method='get'>
                         <button class="btn buttonRowElem" type="submit">Open Report</button>
                     </form>
-                    <form action={% url 'embark-download' analysis_id %} method='get'>
-                        <button class="btn buttonRowElem" type="submit">Download Report</button>
-                    </form>
                     <!-- TODO add another uploader/start pattern for firmware_id-->
                     <form action={% url 'embark-uploader-home' %} method='get'>
                         <button type="submit" class="btn buttonRowElem" >Rescan</button>

embark/templates/dashboard/reportDashboard.html

@@ -115,7 +115,7 @@
                                                     <button type="submit" class="btn buttonRowElem" >Make Zip of EMBA-logs</button>
                                                 </form>
                                             {% endif %}
-                                            <form action={% url 'embark-html-report' firmware.id 'index.html' %} method='get'>
+                                            <form action={% url 'embark-html-report-index' firmware.id 'index.html' %} method='get'>
                                                 <button type="submit" class="btn buttonRowElem" >Open Report</button>
                                             </form>
                                             <form action={% url 'embark-IndividualReportDashboard' firmware.id %} method='get'>

embark/uploader/views.py

@@ -154,12 +154,13 @@ def start_analysis(request):
             new_analysis.firmware_name = os.path.basename(new_analysis.firmware.file.name)
             # save form
             new_analysis = form.save(commit=True)
-            # add labels from devices
+            # add labels from devices FIXME what if device has no label
             devices = form.cleaned_data["device"]
             logger.debug("Got %d devices in this analysis", devices.count())
             for device in devices:
-                logger.debug(" Adding Label=%s", device.device_label.label_name)
-                new_analysis.label.add(device.device_label)
+                if device.device_label:
+                    logger.debug(" Adding Label=%s", device.device_label.label_name)
+                    new_analysis.label.add(device.device_label)
             new_analysis.save()
             logger.debug("new_analysis %s has label: %s", new_analysis, new_analysis.label)
             # inject into bounded Executor