From 4025c4d7b52c8c174ec1d4c02c2a864cce249604 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Thu, 18 Jun 2026 13:48:06 +0800 Subject: [PATCH] fix(security): scope Tauri download_file/upload_file to fs_scope (#4639) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- .../src-tauri/src/transfer_file.rs | 73 ++++++++++++++++++- 1 file changed, 72 insertions(+), 1 deletion(-) diff --git a/apps/readest-app/src-tauri/src/transfer_file.rs b/apps/readest-app/src-tauri/src/transfer_file.rs index ffbf1ea4..f898cb36 100644 --- a/apps/readest-app/src-tauri/src/transfer_file.rs +++ b/apps/readest-app/src-tauri/src/transfer_file.rs @@ -8,7 +8,8 @@ use futures_util::TryStreamExt; use serde::{ser::Serializer, Serialize}; -use tauri::{command, ipc::Channel}; +use tauri::{command, ipc::Channel, AppHandle}; +use tauri_plugin_fs::FsExt; use tokio::{ fs::File, io::{AsyncWriteExt, BufWriter}, @@ -82,6 +83,35 @@ pub enum Error { ContentLength(String), #[error("request failed with status code {0}: {1}")] 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 { @@ -102,7 +132,9 @@ pub struct ProgressPayload { } #[command] +#[allow(clippy::too_many_arguments)] // Tauri command surface mirrors the JS caller's options. pub async fn download_file( + app: AppHandle, url: &str, file_path: &str, headers: HashMap, @@ -115,6 +147,8 @@ pub async fn download_file( use std::cmp::min; use tokio::io::AsyncSeekExt; + ensure_path_allowed(&app, file_path)?; + const PART_SIZE: u64 = 1024 * 1024; let client = reqwest::ClientBuilder::new() @@ -279,12 +313,15 @@ pub async fn download_file( #[command] pub async fn upload_file( + app: AppHandle, url: &str, file_path: &str, method: &str, headers: HashMap, on_progress: Channel, ) -> Result { + ensure_path_allowed(&app, file_path)?; + let file = File::open(file_path).await?; let file_len = file.metadata().await.unwrap().len(); @@ -330,3 +367,37 @@ fn file_to_body(channel: Channel, 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" + )); + } +}