Address review comments: escape JSON strings

koordinates · Jul 11, 2023 · ae121ad · ae121ad
1 parent 16e279b
commit ae121ad
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 11 deletions.
diff --git a/kart/html_diff_writer.py b/kart/html_diff_writer.py
@@ -57,23 +57,28 @@ def write_diff(self, diff_format=DiffFormat.FULL):
         }
 
         fo = resolve_output_path(self.output_path)
-        fo.write(
-            template.substitute(
-                {
-                    "title": html.escape(title),
-                    "geojson_data": json.dumps(
-                        all_datasets_geojson, cls=ExtendedJsonEncoder
-                    ),
-                }
-            )
-        )
+        fo.write(self.substitute_into_template(template, title, all_datasets_geojson))
 
         if fo != sys.stdout:
             fo.close()
             webbrowser.open_new(f"file://{self.output_path.resolve()}")
 
         self.write_warnings_footer()
 
+    @classmethod
+    def substitute_into_template(cls, template, title, all_datasets_geojson):
+        return template.substitute(
+            {
+                "title": html.escape(title),
+                "geojson_data": json.dumps(
+                    all_datasets_geojson, cls=ExtendedJsonEncoder
+                )
+                .replace("/", r"\x2f")
+                .replace("<", r"\x3c")
+                .replace(">", r"\x3e"),
+            }
+        )
+
 
 HtmlDiffWriter.filtered_dataset_deltas_as_geojson = (
     GeojsonDiffWriter.filtered_dataset_deltas_as_geojson

diff --git a/tests/test_diff.py b/tests/test_diff.py
@@ -1,6 +1,7 @@
 import functools
 import json
 import re
+import string
 import time
 from pathlib import Path
 
@@ -10,6 +11,7 @@
 import kart
 from kart.diff_format import DiffFormat
 from kart.diff_structs import Delta, DeltaDiff
+from kart.html_diff_writer import HtmlDiffWriter
 from kart.json_diff_writers import JsonLinesDiffWriter
 from kart.geometry import hex_wkb_to_ogr
 from kart.repo import KartRepo
@@ -2096,14 +2098,47 @@ def test_attached_files_patch(data_archive, cli_runner):
             },
         }
 
+
 def test_load_user_provided_html_template(data_archive, cli_runner):
     with data_archive("points") as repo_path:
         r = cli_runner.invoke(
             [
                 "diff",
                 f"--output-format=html",
-                f"--html-template=" + str(Path(__file__).absolute().parent.parent / "kart" / "diff-view.html"),
+                f"--html-template="
+                + str(
+                    Path(__file__).absolute().parent.parent / "kart" / "diff-view.html"
+                ),
                 "HEAD^...",
             ]
         )
         assert r.exit_code == 0, r.stderr
+
+
+def test_xss_protection():
+    TEMPLATE = """
+<html>
+  <head>
+    <title>Kart Diff: ${title}</title>
+    <script type="application/json">${geojson_data}</script>
+  </head>
+  <body>...</body>
+</html>
+""".lstrip()
+    html_xss = "<script>alert(1);</script>"
+    json_xss = {"key": "</script><script>alert(1);</script>"}
+    result = HtmlDiffWriter.substitute_into_template(
+        string.Template(TEMPLATE), html_xss, json_xss
+    )
+
+    EXPECTED_RESULT = """
+<html>
+  <head>
+    <title>Kart Diff: &lt;script&gt;alert(1);&lt;/script&gt;</title>
+    <script type="application/json">{"key": "\\x3c\\x2fscript\\x3e\\x3cscript\\x3ealert(1);\\x3c\\x2fscript\\x3e"}</script>
+  </head>
+  <body>...</body>
+</html>
+""".lstrip()
+
+    assert result == EXPECTED_RESULT