Skip to content

Commit

Permalink
fix(core): do not allow path traversal on the asset protocol (#3774)
Browse files Browse the repository at this point in the history
  • Loading branch information
lucasfernog committed Mar 28, 2022
1 parent 8661e3e commit 34a402f
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 52 deletions.
5 changes: 5 additions & 0 deletions .changes/fix-asset-protocol-validation.md
@@ -0,0 +1,5 @@
---
"tauri": patch
---

Do not allow path traversal on the asset protocol.
60 changes: 59 additions & 1 deletion core/tauri/src/api/file.rs
Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
1 change: 0 additions & 1 deletion core/tauri/src/api/path.rs
Expand Up @@ -170,7 +170,6 @@ pub fn parse<P: AsRef<Path>>(
}
p.push(component);
}
println!("res {:?}", p);

Ok(p)
}
Expand Down
72 changes: 22 additions & 50 deletions core/tauri/src/endpoints/file_system.rs
Expand Up @@ -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,
};
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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)
}

Expand All @@ -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)
}

Expand All @@ -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()))
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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(())
}

Expand All @@ -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(())
Expand All @@ -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(())
Expand All @@ -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(())
}

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)),
}
}

Expand All @@ -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) {
Expand Down
7 changes: 7 additions & 0 deletions core/tauri/src/manager.rs
Expand Up @@ -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();
Expand All @@ -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);
Expand Down

0 comments on commit 34a402f

Please sign in to comment.