fix(security): scope Tauri download_file/upload_file to fs_scope (#4639)

`download_file` and `upload_file` passed a webview-supplied `file_path`
straight to `File::create`/`File::open` with no validation, so any JS in the
privileged Tauri origin could write or read arbitrary local paths (e.g.
~/.ssh/id_rsa, shell rc files, autostart entries). (GHSA-55vr-pvq5-6fmg)

Validate the path before any file open/create: reject relative paths and `..`
traversal, then require it to be inside the app's filesystem scope
(`fs_scope().is_allowed`), the same mechanism `dir_scanner::read_dir` uses.
Legitimate destinations stay covered — the static capability globs ($APPDATA
/Readest, $APPCACHE, $TEMP) plus persisted dialog grants for custom roots and
external library folders.

AppHandle is injected by Tauri, so the JS invoke surface is unchanged. Adds a
unit test for the traversal/relative-path rejection.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-06-18 13:48:06 +08:00
committed by GitHub
parent 495783d045
commit 4025c4d7b5
@@ -8,7 +8,8 @@
use futures_util::TryStreamExt; use futures_util::TryStreamExt;
use serde::{ser::Serializer, Serialize}; use serde::{ser::Serializer, Serialize};
use tauri::{command, ipc::Channel}; use tauri::{command, ipc::Channel, AppHandle};
use tauri_plugin_fs::FsExt;
use tokio::{ use tokio::{
fs::File, fs::File,
io::{AsyncWriteExt, BufWriter}, io::{AsyncWriteExt, BufWriter},
@@ -82,6 +83,35 @@ pub enum Error {
ContentLength(String), ContentLength(String),
#[error("request failed with status code {0}: {1}")] #[error("request failed with status code {0}: {1}")]
HttpErrorCode(u16, String), HttpErrorCode(u16, String),
#[error("permission denied: path not in filesystem scope: {0}")]
Forbidden(String),
}
/// Reject paths the webview must not be allowed to target: relative paths and
/// any `..` parent-directory traversal. `fs_scope().is_allowed` is a glob match,
/// so a `..` segment could otherwise escape an allowed prefix.
fn has_disallowed_components(file_path: &str) -> bool {
let path = std::path::Path::new(file_path);
!path.is_absolute()
|| path
.components()
.any(|c| matches!(c, std::path::Component::ParentDir))
}
/// Validate a webview-supplied `file_path` before any `File::create`/`File::open`.
/// Without this, `download_file`/`upload_file` would write/read arbitrary local
/// paths (e.g. `~/.ssh/id_rsa`, autostart entries) from any JS running in the
/// privileged Tauri origin — see GHSA-55vr-pvq5-6fmg. We require an absolute,
/// traversal-free path that is inside the app's filesystem scope (the static
/// capability globs plus persisted dialog grants for custom/external roots).
fn ensure_path_allowed(app: &AppHandle, file_path: &str) -> Result<()> {
if has_disallowed_components(file_path) {
return Err(Error::Forbidden(file_path.to_string()));
}
if !app.fs_scope().is_allowed(std::path::Path::new(file_path)) {
return Err(Error::Forbidden(file_path.to_string()));
}
Ok(())
} }
impl Serialize for Error { impl Serialize for Error {
@@ -102,7 +132,9 @@ pub struct ProgressPayload {
} }
#[command] #[command]
#[allow(clippy::too_many_arguments)] // Tauri command surface mirrors the JS caller's options.
pub async fn download_file( pub async fn download_file(
app: AppHandle,
url: &str, url: &str,
file_path: &str, file_path: &str,
headers: HashMap<String, String>, headers: HashMap<String, String>,
@@ -115,6 +147,8 @@ pub async fn download_file(
use std::cmp::min; use std::cmp::min;
use tokio::io::AsyncSeekExt; use tokio::io::AsyncSeekExt;
ensure_path_allowed(&app, file_path)?;
const PART_SIZE: u64 = 1024 * 1024; const PART_SIZE: u64 = 1024 * 1024;
let client = reqwest::ClientBuilder::new() let client = reqwest::ClientBuilder::new()
@@ -279,12 +313,15 @@ pub async fn download_file(
#[command] #[command]
pub async fn upload_file( pub async fn upload_file(
app: AppHandle,
url: &str, url: &str,
file_path: &str, file_path: &str,
method: &str, method: &str,
headers: HashMap<String, String>, headers: HashMap<String, String>,
on_progress: Channel<ProgressPayload>, on_progress: Channel<ProgressPayload>,
) -> Result<String> { ) -> Result<String> {
ensure_path_allowed(&app, file_path)?;
let file = File::open(file_path).await?; let file = File::open(file_path).await?;
let file_len = file.metadata().await.unwrap().len(); let file_len = file.metadata().await.unwrap().len();
@@ -330,3 +367,37 @@ fn file_to_body(channel: Channel<ProgressPayload>, file: File, file_len: u64) ->
}), }),
)) ))
} }
#[cfg(test)]
mod tests {
use super::has_disallowed_components;
#[test]
fn rejects_relative_and_traversal_paths() {
// Relative paths can't be reasoned about against an absolute scope.
assert!(has_disallowed_components("relative/file.epub"));
assert!(has_disallowed_components("file.epub"));
// `..` traversal, whether the path is relative or absolute.
assert!(has_disallowed_components("foo/../bar"));
assert!(has_disallowed_components(
"/home/user/Readest/../../.ssh/id_rsa"
));
}
#[cfg(unix)]
#[test]
fn accepts_plain_absolute_paths() {
assert!(!has_disallowed_components(
"/Users/x/Library/Caches/app/book.epub"
));
assert!(!has_disallowed_components("/Users/x/Readest/Books/h.epub"));
}
#[cfg(windows)]
#[test]
fn accepts_plain_absolute_paths_windows() {
assert!(!has_disallowed_components(
"C:\\Users\\x\\AppData\\Roaming\\Readest\\Books\\h.epub"
));
}
}