third round of security fixing

www/api/resources/multigene_dash_data.py

@@ -10,6 +10,8 @@
 from gear.mg_plotting import PlotError
 from plotly.utils import PlotlyJSONEncoder
 
+from werkzeug.utils import secure_filename
+
 # SAdkins - 2/15/21 - This is a list of datasets already log10-transformed where if selected will use log10 as the default dropdown option
 # This is meant to be a short-term solution until more people specify their data is transformed via the metadata
 LOG10_TRANSFORMED_DATASETS = [
@@ -103,16 +105,18 @@ def create_composite_index_column(df, columns):
 def create_projection_adata(dataset_adata, dataset_id, projection_id):
     # Create AnnData object out of readable CSV file
     # ? Does it make sense to put this in the geardb/Analysis class?
+    projection_id = secure_filename(projection_id)
+    dataset_id = secure_filename(dataset_id)
+
     import scanpy as sc
     projection_dir = Path(PROJECTIONS_BASE_DIR).joinpath("by_dataset", dataset_id)
     # Sanitize input to prevent path traversal
-    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id)).resolve()
-    if not str(projection_adata_path).startswith(str(projection_dir)):
-        raise ValueError("Not allowed.")
+    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id))
+
     if projection_adata_path.is_file():
         return sc.read_h5ad(projection_adata_path)  #, backed="r")
 
-    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id)).resolve()
+    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id))
     try:
         projection_adata = sc.read_csv(projection_csv_path)
     except Exception as e:

www/api/resources/plotly_data.py

@@ -12,6 +12,8 @@
 from flask import request
 from flask_restful import Resource
 
+from werkzeug.utils import secure_filename
+
 from gear.plotting import PlotError, generate_plot, plotly_color_map
 from plotly.utils import PlotlyJSONEncoder
 
@@ -24,16 +26,18 @@
 def create_projection_adata(dataset_adata, dataset_id, projection_id):
     # Create AnnData object out of readable CSV file
     # ? Does it make sense to put this in the geardb/Analysis class?
+    projection_id = secure_filename(projection_id)
+    dataset_id = secure_filename(dataset_id)
+
     import scanpy as sc
     projection_dir = Path(PROJECTIONS_BASE_DIR).joinpath("by_dataset", dataset_id)
     # Sanitize input to prevent path traversal
-    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id)).resolve()
-    if not str(projection_adata_path).startswith(str(projection_dir)):
-        raise ValueError("Not allowed.")
+    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id))
+
     if projection_adata_path.is_file():
         return sc.read_h5ad(projection_adata_path)  # , backed="r")
 
-    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id)).resolve()
+    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id))
     try:
         projection_adata = sc.read_csv(projection_csv_path)
     except:

www/api/resources/svg_data.py

@@ -8,6 +8,8 @@
 from flask import request
 from flask_restful import Resource
 
+from werkzeug.utils import secure_filename
+
 TWO_LEVELS_UP = 2
 abs_path_www = Path(__file__).resolve().parents[TWO_LEVELS_UP] # web-root dir
 PROJECTIONS_BASE_DIR = abs_path_www.joinpath('projections')
@@ -21,15 +23,17 @@ def __init__(self, message="") -> None:
 def create_projection_adata(dataset_adata, dataset_id, projection_id):
     # Create AnnData object out of readable CSV file
     # ? Does it make sense to put this in the geardb/Analysis class?
+    projection_id = secure_filename(projection_id)
+    dataset_id = secure_filename(dataset_id)
+
     projection_dir = Path(PROJECTIONS_BASE_DIR).joinpath("by_dataset", dataset_id)
     # Sanitize input to prevent path traversal
-    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id)).resolve()
-    if not str(projection_adata_path).startswith(str(projection_dir)):
-        raise ValueError("Not allowed.")
+    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id))
+
     if projection_adata_path.is_file():
         return sc.read_h5ad(projection_adata_path)  #, backed="r")
 
-    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id)).resolve()
+    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id))
     try:
         projection_adata = sc.read_csv(projection_csv_path)
     except:

www/api/resources/tsne_data.py

@@ -17,6 +17,8 @@
 from flask_restful import Resource
 from matplotlib import cm
 
+from werkzeug.utils import secure_filename
+
 sc.settings.set_figure_params(dpi=100)
 sc.settings.verbosity = 0
 
@@ -91,15 +93,16 @@ def create_colorscale_with_zero_gray(colorscale):
 def create_projection_adata(dataset_adata, dataset_id, projection_id):
     # Create AnnData object out of readable CSV file
     # ? Does it make sense to put this in the geardb/Analysis class?
+    projection_id = secure_filename(projection_id)
+    dataset_id = secure_filename(dataset_id)
+
     projection_dir = Path(PROJECTIONS_BASE_DIR).joinpath("by_dataset", dataset_id)
     # Sanitize input to prevent path traversal
-    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id)).resolve()
-    if not str(projection_adata_path).startswith(str(projection_dir)):
-        raise ValueError("Not allowed.")
+    projection_adata_path = projection_dir.joinpath("{}.h5ad".format(projection_id))
     if projection_adata_path.is_file():
         return sc.read_h5ad(projection_adata_path)#, backed="r")
 
-    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id)).resolve()
+    projection_csv_path = projection_dir.joinpath("{}.csv".format(projection_id))
     try:
         projection_adata = sc.read_csv(projection_csv_path)
     except Exception as e:

www/cgi/download_weighted_gene_cart.cgi

@@ -9,6 +9,8 @@ import sys
 from shutil import copyfileobj
 from pathlib import Path
 
+from werkzeug.utils import secure_filename
+
 lib_path = Path(__file__).resolve().parents[2].joinpath('lib')
 sys.path.insert(0, str(lib_path))
 
@@ -21,12 +23,13 @@ def main():
 
     form = cgi.FieldStorage()
     share_id = form.getvalue('share_id')
+    share_id = secure_filename(share_id)
 
     if not share_id:
         raise Exception("ERROR: Share ID not provided")
 
     # Get the gene symbols from the shared cart file
-    file_path = Path(CARTS_BASE_DIR).joinpath("{}.tab".format("cart." + share_id)).resolve()
+    file_path = Path(CARTS_BASE_DIR).joinpath("{}.tab".format("cart." + share_id))
     if not str(file_path).startswith(str(CARTS_BASE_DIR)):
         raise ValueError("Not allowed.")
 

www/cgi/save_default_display.cgi

@@ -7,11 +7,15 @@ lib_path = os.path.abspath(os.path.join('..', '..', 'lib'))
 sys.path.append(lib_path)
 import geardb
 import mysql.connector
+from werkzeug.utils import secure_filename
 
 
 def attempt_symlink(cursor, user_id, dataset_id, display_id, is_multigene):
     """Attempt to create a symlink if the user of the saved display is also the dataset owner."""
 
+    dataset_id = secure_filename(dataset_id)
+    display_id = secure_filename(display_id)
+
     DATASET_PREVIEWS_DIR = "/var/www/img/dataset_previews"
 
     gene = "single"
@@ -20,15 +24,6 @@ def attempt_symlink(cursor, user_id, dataset_id, display_id, is_multigene):
 
     filename = os.path.join(DATASET_PREVIEWS_DIR, "{}.{}.png".format(dataset_id, display_id))
 
-    # Normalize path to avoid directory traversal attacks (e.g. ../../../etc/passwd) and validate
-    filename = os.path.normpath(filename)
-    if not filename.startswith(DATASET_PREVIEWS_DIR):
-        print("Invalid filename: {}".format(filename), file=sys.stderr)
-        sys.stdout = original_stdout
-        print('Content-Type: application/json\n\n')
-        print(json.dumps(dict(success=False)))
-        return
-
     if not os.path.isfile(filename):
         print("File to static image does not exist.  Skipping attempted symlink.", file=sys.stderr)
         return

www/js/common.v2.js

@@ -404,11 +404,14 @@ const convertToFormData = (object) => {
 const createToast = (msg, levelClass="is-danger") => {
     const toast = document.createElement("div");
     toast.classList.add("notification", "js-toast", levelClass, "animate__animated", "animate__fadeInUp", "animate__faster");
-    toast.innerHTML = `
-        <button class="delete"></button>
-        ${msg}
-    `;
-
+    const toastButton = document.createElement("button");
+    toastButton.classList.add("delete");
+    toastButton.addEventListener("click", (event) => {
+        const notification = event.target.closest(".js-toast.notification");
+        notification.remove(notification);
+    });
+    toast.appendChild(toastButton);
+    toast.appendChild(document.createTextNode(msg));
 
     const numToasts = document.querySelectorAll(".js-toast.notification").length;
 
@@ -423,12 +426,6 @@ const createToast = (msg, levelClass="is-danger") => {
         document.getElementById("main_c").prepend(toast);
     }
 
-    // This should get the newly added notification since it is now the first
-    toast.querySelector(".js-toast.notification .delete").addEventListener("click", (event) => {
-        const notification = event.target.closest(".js-toast.notification");
-        notification.remove(notification);
-    });
-
     // For a success message, remove it after 3 seconds
     if (levelClass === "is-success") {
         const notification = document.querySelector(".js-toast.notification:last-of-type");

www/js/compare_datasets.v2.js

@@ -171,11 +171,19 @@ const adjustGeneTableLabels = () => {
     const geneFoldchanges = document.getElementById("tbl_gene_foldchanges");
 	const log_base = document.getElementById("log_base").value;
 
+	const spanIcon = document.createElement("span");
+	spanIcon.classList.add("icon");
+	const i = document.createElement("i");
+	i.classList.add("mdi", "mdi-sort-numeric-ascending");
+	i.setAttribute("aria-hidden", "true");
+	spanIcon.appendChild(i);
+	geneFoldchanges.appendChild(spanIcon);
+
 	if (log_base === "raw") {
-		geneFoldchanges.innerHTML = 'Fold Change <span class="icon"><i class="mdi mdi-sort-numeric-ascending" aria-hidden="true"></i></span>';
+		geneFoldchanges.prepend("Fold Change ");
 		return;
 	}
-	geneFoldchanges.innerHTML = `Log${log_base} Fold Change <span class="icon"><i class="mdi mdi-sort-numeric-ascending" aria-hidden="true"></i></span>`;
+	geneFoldchanges.prepend(`Log${log_base} Fold Change `);
 }
 
 const appendGeneTagButton = (geneTagElt) => {

www/js/gene_collection_manager.js

@@ -1106,9 +1106,7 @@ const showGcActionNote = (gcId, shareUrl) => {
     const noteSelector = `#result_gc_id_${gcId} span.js-gc-action-note`;
     const noteSelecterElt = document.querySelector(noteSelector);
 
-    const msg = copyToClipboard(shareUrl) ? "URL copied to clipboard" : `Failed to copy to clipboard. URL: ${shareUrl}`;
-
-    noteSelecterElt.innerHTML = msg;
+    noteSelecterElt.innerHTML = copyToClipboard(shareUrl) ? "URL copied to clipboard" : `Failed to copy to clipboard. URL: ${shareUrl}`;
     noteSelecterElt.classList.remove("is-hidden");
 
     setTimeout(() => {

www/js/plot_display_config.v2.js

@@ -282,7 +282,12 @@ function truncateAxisLabels(plotLayout) {
             ? `${elText.substring(0, TICK_LABEL_TRUNCATION_LEN)}...`
             : elText;
 
-        el.innerHTML = `<a style="fill:inherit;">${sublabel}</a>`;
+
+        const subLabelElt = document.createElement('a');
+        subLabelElt.setAttribute('style', 'fill:inherit;');
+        subLabelElt.textContent = sublabel;
+        el.textContent = "";
+        el.appendChild(subLabelElt);
     }
 
     const axis_ticktexts = {}