From 920925abaeb24a5590784514c77a85562576978c Mon Sep 17 00:00:00 2001
From: oracola91 <118899664+oracola91@users.noreply.github.com>
Date: Sat, 24 Oct 2026 13:31:06 +0300
Subject: [PATCH] PDF export keeps only the links the slide show opens

A denylist can't keep up with OS protocol handlers: ms-msdt:, ms-word:
with a share path, ldap: and file:///%5C%5Chost/share still went into
exported PDFs. PDF export now writes a URL link only when safe_url
allows it, the check the slide show already uses on a click: http,
https and mailto. Links between slides are unchanged, and any other
link's text stays on the page without the link.

safe_url moves from deckcraft-anim to deckcraft-model, which both
crates use; deckcraft_anim::safe_url stays as a re-export. Local file:
and relative links are left out for now, as asked in review.
---
 crates/anim/src/show.rs   | 14 ++------------
 crates/model/src/tests.rs | 24 ++++++++++++++++++++++++
 crates/model/src/text.rs  | 13 +++++++++++++
 crates/pdf/src/lib.rs     | 33 ++++-----------------------------
 crates/pdf/src/tests.rs   |  8 +++++++-
 5 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/crates/anim/src/show.rs b/crates/anim/src/show.rs
index a1117c62..403f05a0 100644
--- a/crates/anim/src/show.rs
+++ b/crates/anim/src/show.rs
@@ -29,18 +29,8 @@ pub enum ShowAction {
     Exit,
 }
 
-/// Can a link from a (possibly untrusted) deck be opened outside the show? Only web pages and
-/// mail: `file:`, `javascript:`, relative paths and other schemes (OS protocol handlers) are not
-/// opened on a click.
-pub fn safe_url(url: &str) -> bool {
-    let u = url.trim();
-    let Some((scheme, rest)) = u.split_once(':') else { return false };
-    match scheme.to_ascii_lowercase().as_str() {
-        "http" | "https" => rest.starts_with("//") && rest.len() > 2,
-        "mailto" => !rest.is_empty(),
-        _ => false,
-    }
-}
+// Which links the show opens on a click; PDF export writes the same ones.
+pub use deckcraft_model::text::safe_url;
 
 /// What a clicked link does in the show (`ShowState::follow`).
 #[derive(Clone, Debug, PartialEq, Eq)]
diff --git a/crates/model/src/tests.rs b/crates/model/src/tests.rs
index bc3817d9..4019a117 100644
--- a/crates/model/src/tests.rs
+++ b/crates/model/src/tests.rs
@@ -294,3 +294,27 @@ fn table_text_takes_the_masters_other_style_and_shapes_the_default_text_style()
     assert_eq!(pt(&Shape { text: Some(body.clone()), ..Default::default() }), Some(26.0), "a shape");
     assert_eq!(pt(&Shape { text: Some(body), text_box: true, ..Default::default() }), Some(26.0), "a text box");
 }
+
+#[test]
+fn only_web_and_mail_links_are_safe_to_open() {
+    use crate::text::safe_url;
+    for url in ["https://example.com", " HTTP://example.com/x", "mailto:someone@example.com"] {
+        assert!(safe_url(url), "{url}");
+    }
+    let refused = [
+        "https://",
+        "mailto:",
+        "file:///C:/notes.pdf",
+        "notes.pdf",
+        "javascript:alert(1)",
+        "ms-msdt:/id PCWDiagnostic",
+        "ldap://attacker.example/x",
+        "//attacker.example/share",
+        "\\\\attacker.example\\share",
+        "smb://attacker.example/share",
+        "search-ms:query=x",
+    ];
+    for url in refused {
+        assert!(!safe_url(url), "{url}");
+    }
+}
diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs
index 4b814129..17d606de 100644
--- a/crates/model/src/text.rs
+++ b/crates/model/src/text.rs
@@ -328,6 +328,19 @@ pub enum Action {
     PlayMedia,
 }
 
+/// Can a link from a (possibly untrusted) deck be opened outside it, by the slide show or by
+/// whoever opens an exported file? Only web pages and mail: `file:`, `javascript:`, relative paths
+/// and other schemes (OS protocol handlers, network shares) are not.
+pub fn safe_url(url: &str) -> bool {
+    let u = url.trim();
+    let Some((scheme, rest)) = u.split_once(':') else { return false };
+    match scheme.to_ascii_lowercase().as_str() {
+        "http" | "https" => rest.starts_with("//") && rest.len() > 2,
+        "mailto" => !rest.is_empty(),
+        _ => false,
+    }
+}
+
 fn custom_show_returns() -> bool {
     true
 }
diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs
index a50d0f21..1b80ad6a 100644
--- a/crates/pdf/src/lib.rs
+++ b/crates/pdf/src/lib.rs
@@ -17,7 +17,7 @@ use craft_pdf::write::{self, Metadata, Writer, backend as krilla};
 
 use deckcraft_fonts::FontDb;
 use deckcraft_model::Presentation;
-use deckcraft_model::text::Action as ClickAction;
+use deckcraft_model::text::{Action as ClickAction, safe_url};
 use deckcraft_render::{PlacedText, RenderOpts};
 use krilla::action::{Action, LinkAction};
 use krilla::annotation::{Annotation, LinkAnnotation, Target};
@@ -565,7 +565,9 @@ impl Exporter<'_> {
                 slide.and_then(|s| page_of.get(&s)).map(|p| Target::Destination(XyzDestination::new(*p, Point::from_xy(0.0, 0.0)).into()))
             };
             let target = match &l.link.action {
-                ClickAction::Url { url } if exportable_url(url) => Some(Target::Action(Action::Link(LinkAction::new(url.clone())))),
+                // Only the links the slide show opens (web and mail): a reader of the file gets the rest
+                // as plain text.
+                ClickAction::Url { url } if safe_url(url) => Some(Target::Action(Action::Link(LinkAction::new(url.clone())))),
                 ClickAction::Slide { slide } => goto(pres.slides.iter().position(|x| x.id == *slide)),
                 ClickAction::NextSlide => goto(Some(i + 1)),
                 ClickAction::PreviousSlide => goto(i.checked_sub(1)),
@@ -693,32 +695,5 @@ fn to_path(bp: &kurbo::BezPath) -> Option<krilla::geom::Path> {
     pb.finish()
 }
 
-/// Can a hyperlink from a (possibly untrusted) deck become a PDF link? Scripts (`javascript:`,
-/// `vbscript:`), `data:` and paths to another machine (`\\host\share`, `//host/share`,
-/// `file://host/...`, `smb://host/...`, and Windows Search's `search:` and `search-ms:`, whose
-/// `crumb=location:` can name one) cannot: opening one connects to that host, which on Windows
-/// sends the user's NTLM credentials. Web, mail, local `file:` and relative links can. Read the way
-/// a browser reads a URL: control characters dropped, leading spaces trimmed, any case.
-fn exportable_url(url: &str) -> bool {
-    let u: String = url.chars().filter(|c| !c.is_ascii_control()).collect();
-    let u = u.trim_start().replace('\\', "/");
-    if u.starts_with("//") {
-        return false;
-    }
-    let Some((scheme, rest)) = u.split_once(':') else { return true };
-    match scheme.to_ascii_lowercase().as_str() {
-        "javascript" | "vbscript" | "data" | "smb" | "search" | "search-ms" => false,
-        "file" => match rest.strip_prefix("//") {
-            // `file:///C:/...` and `file://localhost/...` are local; `file://host/...` and `file:////host/...` are not.
-            Some(p) => {
-                let (host, path) = p.split_once('/').unwrap_or((p, ""));
-                (host.is_empty() || host.eq_ignore_ascii_case("localhost")) && !path.starts_with('/')
-            }
-            None => true,
-        },
-        _ => true,
-    }
-}
-
 #[cfg(test)]
 mod tests;
diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs
index 05609ce7..f29935f6 100644
--- a/crates/pdf/src/tests.rs
+++ b/crates/pdf/src/tests.rs
@@ -138,8 +138,14 @@ fn unsafe_url_links_are_not_exported() {
     use deckcraft_model::text::{Action, Hyperlink};
     use deckcraft_model::{Shape, ShapeId, Xfrm};
     use hayro_syntax::object::{Array, Dict, String as PdfString};
-    let safe = ["https://example.com/deck", "mailto:someone@example.com", "file:///C:/talks/notes.pdf", "notes.pdf"];
+    let safe = ["https://example.com/deck", "mailto:someone@example.com"];
     let refused = [
+        "file:///C:/talks/notes.pdf",
+        "notes.pdf",
+        "ms-msdt:/id PCWDiagnostic /skip force",
+        "ms-word:ofe|u|\\\\attacker.example\\share\\x.docx",
+        "ldap://attacker.example/x",
+        "file:///%5C%5Cattacker.example/share/x",
         "javascript:alert(1)",
         "\t JavaScript:alert(1)",
         "java\nscript:alert(1)",