fix(core): do not allow path traversal on the asset protocol (#3774)

Author: lucasfernog

.changes/fix-asset-protocol-validation.md

@@ -0,0 +1,5 @@
+---
+"tauri": patch
+---
+
+Do not allow path traversal on the asset protocol.

core/tauri/src/api/file.rs

@@ -8,12 +8,57 @@
 mod extract;
 mod file_move;
 
-use std::{fs, path::Path};
+use std::{
+  fs,
+  path::{Display, Path},
+};
 
 #[cfg(feature = "fs-extract-api")]
 pub use extract::*;
 pub use file_move::*;
 
+use serde::{de::Error as DeError, Deserialize, Deserializer};
+
+#[derive(Clone, Debug)]
+pub(crate) struct SafePathBuf(std::path::PathBuf);
+
+impl SafePathBuf {
+  pub fn new(path: std::path::PathBuf) -> Result<Self, &'static str> {
+    if path
+      .components()
+      .any(|x| matches!(x, std::path::Component::ParentDir))
+    {
+      Err("cannot traverse directory, rewrite the path without the use of `../`")
+    } else {
+      Ok(Self(path))
+    }
+  }
+
+  pub unsafe fn new_unchecked(path: std::path::PathBuf) -> Self {
+    Self(path)
+  }
+
+  pub fn display(&self) -> Display<'_> {
+    self.0.display()
+  }
+}
+
+impl AsRef<Path> for SafePathBuf {
+  fn as_ref(&self) -> &Path {
+    self.0.as_ref()
+  }
+}
+
+impl<'de> Deserialize<'de> for SafePathBuf {
+  fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+  where
+    D: Deserializer<'de>,
+  {
+    let path = std::path::PathBuf::deserialize(deserializer)?;
+    SafePathBuf::new(path).map_err(DeError::custom)
+  }
+}
+
 /// Reads the entire contents of a file into a string.
 pub fn read_string<P: AsRef<Path>>(file: P) -> crate::api::Result<String> {
   fs::read_to_string(file).map_err(Into::into)
@@ -28,6 +73,19 @@ pub fn read_binary<P: AsRef<Path>>(file: P) -> crate::api::Result<Vec<u8>> {
 mod test {
   use super::*;
   use crate::api::Error;
+  use quickcheck::{Arbitrary, Gen};
+
+  use std::path::PathBuf;
+
+  impl Arbitrary for super::SafePathBuf {
+    fn arbitrary(g: &mut Gen) -> Self {
+      Self(PathBuf::arbitrary(g))
+    }
+
+    fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
+      Box::new(self.0.shrink().map(SafePathBuf))
+    }
+  }
 
   #[test]
   fn check_read_string() {

core/tauri/src/api/path.rs

@@ -170,7 +170,6 @@ pub fn parse<P: AsRef<Path>>(
     }
     p.push(component);
   }
-  println!("res {:?}", p);
 
   Ok(p)
 }

core/tauri/src/endpoints/file_system.rs

@@ -3,7 +3,11 @@
 // SPDX-License-Identifier: MIT
 
 use crate::{
-  api::{dir, file, path::BaseDirectory},
+  api::{
+    dir,
+    file::{self, SafePathBuf},
+    path::BaseDirectory,
+  },
   scope::Scopes,
   Config, Env, Manager, PackageInfo, Runtime, Window,
 };
@@ -26,29 +30,6 @@ use std::{
   sync::Arc,
 };
 
-#[derive(Clone, Debug)]
-pub struct SafePathBuf(std::path::PathBuf);
-
-impl AsRef<Path> for SafePathBuf {
-  fn as_ref(&self) -> &Path {
-    self.0.as_ref()
-  }
-}
-
-impl<'de> Deserialize<'de> for SafePathBuf {
-  fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-  where
-    D: Deserializer<'de>,
-  {
-    let path = std::path::PathBuf::deserialize(deserializer)?;
-    if path.components().any(|x| matches!(x, Component::ParentDir)) {
-      Err(DeError::custom("cannot traverse directory"))
-    } else {
-      Ok(SafePathBuf(path))
-    }
-  }
-}
-
 /// The options for the directory functions on the file system API.
 #[derive(Debug, Clone, Deserialize)]
 pub struct DirOperationOptions {
@@ -71,7 +52,7 @@ pub struct FileOperationOptions {
 /// The API descriptor.
 #[derive(Deserialize, CommandModule)]
 #[serde(tag = "cmd", rename_all = "camelCase")]
-pub enum Cmd {
+pub(crate) enum Cmd {
   /// The read binary file API.
   ReadFile {
     path: SafePathBuf,
@@ -138,7 +119,7 @@ impl Cmd {
       options.and_then(|o| o.dir),
     )?;
     file::read_binary(&resolved_path)
-      .with_context(|| format!("path: {}", resolved_path.0.display()))
+      .with_context(|| format!("path: {}", resolved_path.display()))
       .map_err(Into::into)
   }
 
@@ -156,7 +137,7 @@ impl Cmd {
       options.and_then(|o| o.dir),
     )?;
     file::read_string(&resolved_path)
-      .with_context(|| format!("path: {}", resolved_path.0.display()))
+      .with_context(|| format!("path: {}", resolved_path.display()))
       .map_err(Into::into)
   }
 
@@ -175,7 +156,7 @@ impl Cmd {
       options.and_then(|o| o.dir),
     )?;
     File::create(&resolved_path)
-      .with_context(|| format!("path: {}", resolved_path.0.display()))
+      .with_context(|| format!("path: {}", resolved_path.display()))
       .map_err(Into::into)
       .and_then(|mut f| f.write_all(&contents).map_err(|err| err.into()))
   }
@@ -199,7 +180,7 @@ impl Cmd {
       dir,
     )?;
     dir::read_dir(&resolved_path, recursive)
-      .with_context(|| format!("path: {}", resolved_path.0.display()))
+      .with_context(|| format!("path: {}", resolved_path.display()))
       .map_err(Into::into)
   }
 
@@ -230,7 +211,7 @@ impl Cmd {
       None => (source, destination),
     };
     fs::copy(src.clone(), dest.clone())
-      .with_context(|| format!("source: {}, dest: {}", src.0.display(), dest.0.display()))?;
+      .with_context(|| format!("source: {}, dest: {}", src.display(), dest.display()))?;
     Ok(())
   }
 
@@ -254,10 +235,10 @@ impl Cmd {
     )?;
     if recursive {
       fs::create_dir_all(&resolved_path)
-        .with_context(|| format!("path: {}", resolved_path.0.display()))?;
+        .with_context(|| format!("path: {}", resolved_path.display()))?;
     } else {
       fs::create_dir(&resolved_path)
-        .with_context(|| format!("path: {} (non recursive)", resolved_path.0.display()))?;
+        .with_context(|| format!("path: {} (non recursive)", resolved_path.display()))?;
     }
 
     Ok(())
@@ -283,10 +264,10 @@ impl Cmd {
     )?;
     if recursive {
       fs::remove_dir_all(&resolved_path)
-        .with_context(|| format!("path: {}", resolved_path.0.display()))?;
+        .with_context(|| format!("path: {}", resolved_path.display()))?;
     } else {
       fs::remove_dir(&resolved_path)
-        .with_context(|| format!("path: {} (non recursive)", resolved_path.0.display()))?;
+        .with_context(|| format!("path: {} (non recursive)", resolved_path.display()))?;
     }
 
     Ok(())
@@ -306,7 +287,7 @@ impl Cmd {
       options.and_then(|o| o.dir),
     )?;
     fs::remove_file(&resolved_path)
-      .with_context(|| format!("path: {}", resolved_path.0.display()))?;
+      .with_context(|| format!("path: {}", resolved_path.display()))?;
     Ok(())
   }
 
@@ -337,7 +318,7 @@ impl Cmd {
       None => (old_path, new_path),
     };
     fs::rename(&old, &new)
-      .with_context(|| format!("old: {}, new: {}", old.0.display(), new.0.display()))
+      .with_context(|| format!("old: {}, new: {}", old.display(), new.display()))
       .map_err(Into::into)
   }
 }
@@ -354,15 +335,18 @@ fn resolve_path<R: Runtime>(
   match crate::api::path::resolve_path(config, package_info, env, &path, dir) {
     Ok(path) => {
       if window.state::<Scopes>().fs.is_allowed(&path) {
-        Ok(SafePathBuf(path))
+        Ok(
+          // safety: the path is resolved by Tauri so it is safe
+          unsafe { SafePathBuf::new_unchecked(path) },
+        )
       } else {
         Err(anyhow::anyhow!(
           crate::Error::PathNotAllowed(path).to_string()
         ))
       }
     }
     Err(e) => super::Result::<SafePathBuf>::Err(e.into())
-      .with_context(|| format!("path: {}, base dir: {:?}", path.0.display(), dir)),
+      .with_context(|| format!("path: {}, base dir: {:?}", path.display(), dir)),
   }
 }
 
@@ -372,18 +356,6 @@ mod tests {
 
   use quickcheck::{Arbitrary, Gen};
 
-  use std::path::PathBuf;
-
-  impl Arbitrary for super::SafePathBuf {
-    fn arbitrary(g: &mut Gen) -> Self {
-      Self(PathBuf::arbitrary(g))
-    }
-
-    fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
-      Box::new(self.0.shrink().map(SafePathBuf))
-    }
-  }
-
   impl Arbitrary for BaseDirectory {
     fn arbitrary(g: &mut Gen) -> Self {
       if bool::arbitrary(g) {

core/tauri/src/manager.rs

@@ -498,6 +498,7 @@ impl<R: Runtime> WindowManager<R> {
 
     #[cfg(protocol_asset)]
     if !registered_scheme_protocols.contains(&"asset".into()) {
+      use crate::api::file::SafePathBuf;
       use tokio::io::{AsyncReadExt, AsyncSeekExt};
       use url::Position;
       let asset_scope = self.state().get::<crate::Scopes>().asset_protocol.clone();
@@ -512,6 +513,12 @@ impl<R: Runtime> WindowManager<R> {
           .decode_utf8_lossy()
           .to_string();
 
+        if let Err(e) = SafePathBuf::new(path.clone().into()) {
+          #[cfg(debug_assertions)]
+          eprintln!("asset protocol path \"{}\" is not valid: {}", path, e);
+          return HttpResponseBuilder::new().status(403).body(Vec::new());
+        }
+
         if !asset_scope.is_allowed(&path) {
           #[cfg(debug_assertions)]
           eprintln!("asset protocol not configured to allow the path: {}", path);