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:
@@ -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<String, String>,
|
||||
@@ -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<String, String>,
|
||||
on_progress: Channel<ProgressPayload>,
|
||||
) -> Result<String> {
|
||||
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<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"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user