Security hardening for upload app

upload/import_task_factories/import_task_factory.py

@@ -1,3 +1,4 @@
+import importlib
 import inspect
 from abc import ABC, abstractmethod
 from typing import Iterable, Type
@@ -41,7 +42,7 @@ def create_import_task(self, upload_pipeline: UploadPipeline):
 def get_import_task_factories() -> list[ImportTaskFactory]:
     # Import all factory scripts into scope  so __subclasses__ works
     for i in settings.IMPORT_TASK_FACTORY_IMPORTS:
-        exec(f"import {i}")
+        importlib.import_module(i)
 
     factories = []
     for itf_class in get_all_subclasses(ImportTaskFactory):

upload/vcf/vcf_preprocess.py

@@ -144,9 +144,13 @@ def preprocess_vcf(upload_step, annotate_gnomad_af=False, disable_swap=False):
     # Split up the VCF
     split_file_rows = upload_step.split_file_rows or settings.VCF_IMPORT_FILE_SPLIT_ROWS
     split_vcf_dir = upload_pipeline.get_pipeline_processing_subdir("split_vcf")
+    # Pass paths via env vars rather than interpolating them into the shell --filter string,
+    # so that metacharacters in paths cannot affect shell parsing.
+    # split runs the filter via sh -c; $VG_HEADER_FILE/$VG_SPLIT_VCF_DIR are expanded there.
+    split_env = {**os.environ, 'VG_HEADER_FILE': cleaned_vcf_header_filename, 'VG_SPLIT_VCF_DIR': split_vcf_dir}
     pipe_commands[SPLIT_VCF_SUB_STEP] = ["split", "-", vcf_name, "--additional-suffix=.vcf.gz", "--numeric-suffixes",
                                          "--lines", str(split_file_rows),
-                                         f"--filter='bash -c \"set -eo pipefail; {{ cat {cleaned_vcf_header_filename}; cat; }} | bgzip -c > {split_vcf_dir}/$FILE\"'"]
+                                         "--filter='bash -c \"set -eo pipefail; { cat $VG_HEADER_FILE; cat; } | bgzip -c > $VG_SPLIT_VCF_DIR/$FILE\"'"]
 
     for sub_step_name in norm_substep_names:
         sub_step_commands = pipe_commands[sub_step_name]
@@ -167,6 +171,7 @@ def preprocess_vcf(upload_step, annotate_gnomad_af=False, disable_swap=False):
             executable="/bin/bash",  # pipefail requires bash, not sh
             stdout=PIPE,
             stderr=PIPE,
+            env=split_env,
         )
         p_stdout, p_stderr = p.communicate()
         logging.info("single command pipe/shell completed - return code: %d", p.returncode)
@@ -193,6 +198,8 @@ def preprocess_vcf(upload_step, annotate_gnomad_af=False, disable_swap=False):
                       "stderr": stderr_f}
             if p:
                 kwargs["stdin"] = p.stdout
+            if sub_step_name == SPLIT_VCF_SUB_STEP:
+                kwargs["env"] = split_env
             p = Popen(cmd, **kwargs)
             pipes[sub_step_name] = p
             stderr_filenames[sub_step_name] = stderr_filename

upload/views/views.py

@@ -62,15 +62,15 @@ def _get_basic_uploaded_file_context(uploaded_file) -> dict:
     if upload_data:
         try:
             data["upload_data"] = upload_data.get_upload_context()
-        except:
+        except Exception:
             pass
     return data
 
 
 def uploadedfile_dict(uploaded_file) -> dict:
     try:
         size = uploaded_file.uploaded_file.size
-    except:
+    except Exception:
         size = None
 
     time_since = timesince(uploaded_file.created)
@@ -101,12 +101,12 @@ def uploadedfile_dict(uploaded_file) -> dict:
                         data['remaining_annotation_runs'] = get_remaining_annotation_runs(uploaded_vcf, upload_pipeline.genome_build)
                     except ObjectDoesNotExist:
                         pass
-        except:
+        except Exception:
             pass  # Genome build is optional
 
         status = upload_pipeline.status
         url = reverse('view_upload_pipeline', kwargs={'upload_pipeline_id': upload_pipeline.pk})
-    except:
+    except Exception:
         status = ProcessingStatus.ERROR
         url = reverse('view_uploaded_file', kwargs={'uploaded_file_id': uploaded_file.pk})
 
@@ -162,7 +162,7 @@ def jfu_upload(request):
     except Exception as e:
         logging.error(e)
         log_traceback()
-        file_dict = {"error": str(e)}
+        file_dict = {"error": "Upload failed. Please try again or contact support."}
 
     logging.debug("views.upload() END")
     return UploadResponse(request, file_dict)