Render RFD PDFs in Asciidoctor more safely

rfd-processor/src/content/asciidoc.rs

@@ -48,22 +48,58 @@ async fn render_pdf(
     .await?;
     write_file(&shim_path, MERMAID_CONFIG_SHIM.as_bytes()).await?;
 
+    // RFD content is untrusted. The CLI defaults to the unsafe safe mode, so run
+    // in safe mode jailed to the content directory, and lock the theme and font
+    // attributes (which asciidoctor-pdf does not jail) so the document cannot
+    // point them elsewhere on disk. The renderer also only receives the
+    // environment it needs to locate Ruby, Node and Chromium.
     let mut command = Command::new("asciidoctor-pdf");
-    command.current_dir(content_dir.clone()).args([
-        "-o",
-        "-",
-        "-r",
-        "base64",
-        "-r",
-        "asciidoctor-mermaid/pdf",
-        "-r",
-        shim_path.to_str().unwrap(),
-        "-a",
-        "reproducible",
-        "-a",
-        "source-highlighter=rouge",
-        file_path.to_str().unwrap(),
-    ]);
+    command
+        .current_dir(content_dir.clone())
+        .env_clear()
+        .envs(std::env::vars_os().filter(|(key, _)| {
+            key.to_str().is_some_and(|key| {
+                ["PATH", "HOME", "TMPDIR", "LANG", "TZ"].contains(&key)
+                    || [
+                        "LC_",
+                        "GEM_",
+                        "BUNDLE_",
+                        "RUBY",
+                        "NODE_",
+                        "PUPPETEER_",
+                        "XDG_",
+                    ]
+                    .iter()
+                    .any(|prefix| key.starts_with(prefix))
+            })
+        }))
+        .args([
+            "-S",
+            "safe",
+            "-B",
+            content_dir.to_str().unwrap(),
+            "-a",
+            "pdf-theme!",
+            "-a",
+            "pdf-themesdir!",
+            "-a",
+            "pdf-fontsdir!",
+        ])
+        .args([
+            "-o",
+            "-",
+            "-r",
+            "base64",
+            "-r",
+            "asciidoctor-mermaid/pdf",
+            "-r",
+            shim_path.to_str().unwrap(),
+            "-a",
+            "reproducible",
+            "-a",
+            "source-highlighter=rouge",
+            file_path.to_str().unwrap(),
+        ]);
 
     let cmd_output = tokio::task::spawn_blocking(move || command.output()).await?;
 

rfd-processor/src/content/mod.rs

@@ -2,7 +2,13 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use std::{borrow::Cow, env, io, path::PathBuf, str::Utf8Error, string::FromUtf8Error};
+use std::{
+    borrow::Cow,
+    env, io,
+    path::{Component, Path, PathBuf},
+    str::Utf8Error,
+    string::FromUtf8Error,
+};
 
 use async_trait::async_trait;
 use base64::DecodeError;
@@ -149,14 +155,15 @@ impl<'a> RenderableRfd<'a> {
             .await?;
 
         for document in documents {
-            let document_path = storage_path.join(
-                document
-                    .path
-                    .replace(dir.trim_start_matches('/'), "")
-                    .trim_start_matches('/'),
-            );
-
-            let path = document_path;
+            let Some(relative_path) = supporting_document_path(&dir, &document.path) else {
+                tracing::warn!(
+                    document.path,
+                    "Skipping supporting document with unsafe path"
+                );
+                continue;
+            };
+
+            let path = storage_path.join(relative_path);
             write_file(&path, &decode_base64(&document.content)?).await?;
 
             tracing::info!(?path, "Wrote supporting document",);
@@ -194,6 +201,19 @@ impl<'a> RenderableRfd<'a> {
     }
 }
 
+/// Returns `repo_path` relative to the RFD directory `dir`,
+/// or `None` if it is not strictly inside `dir` (e.g. it contains `..`).
+/// The result is safe to join onto the render directory.
+fn supporting_document_path<'a>(dir: &str, repo_path: &'a str) -> Option<&'a Path> {
+    let relative = Path::new(repo_path).strip_prefix(dir).ok()?;
+    let is_contained = !relative.as_os_str().is_empty()
+        && relative
+            .components()
+            .all(|component| matches!(component, Component::Normal(_)));
+
+    is_contained.then_some(relative)
+}
+
 impl<'a> RfdDocument for RenderableRfd<'a> {
     fn get_title(&self) -> Option<&str> {
         RfdDocument::get_title(&self.content)
@@ -287,3 +307,48 @@ pub enum RfdOutputError {
     #[error(transparent)]
     Io(#[from] std::io::Error),
 }
+
+#[cfg(test)]
+mod tests {
+    use std::path::Path;
+
+    use super::supporting_document_path;
+
+    #[test]
+    fn test_supporting_document_path_keeps_rfd_relative_path() {
+        assert_eq!(
+            supporting_document_path("rfd/0123", "rfd/0123/example.csv"),
+            Some(Path::new("example.csv"))
+        );
+        assert_eq!(
+            supporting_document_path("rfd/0123", "rfd/0123/images/rfd/0123/diagram.svg"),
+            Some(Path::new("images/rfd/0123/diagram.svg"))
+        );
+    }
+
+    #[test]
+    fn test_supporting_document_path_rejects_escaping_paths() {
+        // Removing every occurrence of `rfd/0123` would turn this into `../../evil`.
+        let spliced = "rfd/0123/.rfd/0123./.rfd/0123./evil";
+        assert_eq!(
+            spliced.replace("rfd/0123", "").trim_start_matches('/'),
+            "../../evil"
+        );
+        assert_eq!(
+            supporting_document_path("rfd/0123", spliced),
+            Some(Path::new(".rfd/0123./.rfd/0123./evil"))
+        );
+
+        assert_eq!(
+            supporting_document_path("rfd/0123", "rfd/0123/../evil"),
+            None
+        );
+        assert_eq!(
+            supporting_document_path("rfd/0123", "rfd/0123/a/../../evil"),
+            None
+        );
+        assert_eq!(supporting_document_path("rfd/0123", "/etc/passwd"), None);
+        assert_eq!(supporting_document_path("rfd/0123", "rfd/0124/evil"), None);
+        assert_eq!(supporting_document_path("rfd/0123", "rfd/0123"), None);
+    }
+}