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 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"
|
||||||
|
));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user