fix: escape user-controlled URLs in ImageFrame and Video embeddings

The |src= option (and Video |width=) were read from options_raw/args_raw
and interpolated into HTML attributes unescaped, allowing stored XSS via
attribute breakout (e.g. src=http://x" onerror="alert(1)).

Route src/href through mistune's escape_url(safe_url(...)) to reject
harmful schemes and percent-encode the value, and HTML-escape width.
Adds regression tests for both embeddings.

Author: redimp

otterwiki/renderer_embeddings.py

@@ -34,6 +34,22 @@ def _style_attr(styles):
     return f' style="{mistune.escape(";".join(styles))}"'
 
 
+# a bare renderer instance, only used for its safe_url() helper below
+_url_safety = mistune.HTMLRenderer()
+
+
+def _safe_attr_url(url):
+    """Return *url* safe to interpolate into an HTML src/href attribute.
+
+    Harmful protocols (``javascript:``, ``data:``, ...) are neutralised and
+    the value is escaped so a crafted value (e.g. an ImageFrame/Video
+    ``|src=`` read from ``options_raw``) cannot break out of the attribute
+    and inject event handlers. Mirrors OtterwikiMdRenderer's handling of
+    link and image URLs.
+    """
+    return mistune.escape_url(_url_safety.safe_url(url))
+
+
 class DatatableEmbedding:
     @hookimpl
     def info(self):
@@ -575,9 +591,10 @@ def embedding_render(
                     )
                 alt = alt or mistune.escape(att_filename)
                 img_url = attachment.get_url()
+            safe_img_url = _safe_attr_url(img_url)
             content += (
-                f'<a href="{img_url}" target="_blank">'
-                f'<img src="{img_url}" alt="{alt}" style="width:100%">'
+                f'<a href="{safe_img_url}" target="_blank">'
+                f'<img src="{safe_img_url}" alt="{alt}" style="width:100%">'
                 f'</a>'
             )
         else:
@@ -681,7 +698,9 @@ def embedding_render(
     ):
         if embedding.lower() != "video":
             return None
-        width = args.options_raw.get("width", "100%")
+        # width is read raw and interpolated into an attribute; escape it so
+        # a crafted value cannot break out and inject event handlers
+        width = mistune.escape(args.options_raw.get("width", "100%"))
         flags = []
         if args.get_flag("controls", True):
             flags.append("controls")
@@ -746,7 +765,7 @@ def embedding_render(
                     t = " type=\"video/mp4\""
                 elif s.endswith(".ogg"):
                     t = " type=\"video/ogg\""
-                video_sources += f'<source src="{s}"{t}>\n'
+                video_sources += f'<source src="{_safe_attr_url(s)}"{t}>\n'
 
         if video_sources:
             html_parts.append(

tests/test_embeddings.py

@@ -184,6 +184,58 @@ def test_embedding_style_no_xss():
         assert """ in html
 
 
+def _assert_no_event_handlers(html):
+    """Fail if any rendered tag carries an on* event-handler attribute."""
+    soup = BeautifulSoup(html, "html.parser")
+    for tag in soup.find_all(True):
+        for attr in tag.attrs:
+            assert not attr.lower().startswith(
+                "on"
+            ), f"event handler {attr!r} injected on <{tag.name}>: {html!r}"
+
+
+def test_imageframe_src_no_xss():
+    """A crafted |src= must not break out of the attribute and inject an
+    event handler (see the ImageFrame/Video XSS report)."""
+    payloads = [
+        'http://x" autofocus onfocus="alert(document.domain)',
+        'https://e.com/x.png" onerror="alert(1)',
+    ]
+    for payload in payloads:
+        md = f"{{{{ImageFrame\n|src={payload}\n}}}}\n"
+        html, _, _ = render.markdown(md)
+        _assert_no_event_handlers(html)
+        # the crafted double-quote must not survive as a live attribute
+        # delimiter (it is neutralised via escaping/percent-encoding)
+        assert 'onfocus="' not in html
+        assert 'onerror="' not in html
+
+
+def test_video_src_no_xss():
+    """A crafted Video src (option or positional) must not inject an event
+    handler onto the <source> element."""
+    payloads = [
+        '{{Video\n|src=https://e.com/v.mp4" onerror="alert(1)\n}}\n',
+        '{{Video\nhttps://e.com/v.mp4" onerror="alert(1)\n}}\n',
+    ]
+    for md in payloads:
+        html, _, _ = render.markdown(md)
+        _assert_no_event_handlers(html)
+
+
+def test_video_width_no_xss():
+    """A crafted |width= must not break out of the attribute — on the file
+    <video> element or on the YouTube <iframe>."""
+    payloads = [
+        '{{Video\n|width=100" onmouseover="alert(1)\n/x.mp4\n}}\n',
+        '{{Video\n|width=100" onmouseover="alert(1)\n'
+        'https://youtu.be/dQw4w9WgXcQ\n}}\n',
+    ]
+    for md in payloads:
+        html, _, _ = render.markdown(md)
+        _assert_no_event_handlers(html)
+
+
 def test_imageframe_alias():
     md = """
 {{Image Frame