diff --git a/changelog/v0.1.0-beta4.txt b/changelog/v0.1.0-beta4.txt index 59479dde..6a4050a3 100644 --- a/changelog/v0.1.0-beta4.txt +++ b/changelog/v0.1.0-beta4.txt @@ -44,6 +44,7 @@ SYNC - A new device that starts from a snapshot whose data can't be read now starts from an older snapshot instead of failing the sync. - Linked devices: allowing or declining remote control now takes effect whichever profile is open. - Linked devices reach a Flow server or WebDAV folder on your local network directly, even when a system proxy is set. +- Linked devices only ever touch files inside the group's own WebDAV folder. - Linked devices sign in to your Flow server about once every 15 minutes instead of before every sync. MUSIC diff --git a/src-tauri/src/sync2/snapshot.rs b/src-tauri/src/sync2/snapshot.rs index b0ceef71..3f194c1c 100644 --- a/src-tauri/src/sync2/snapshot.rs +++ b/src-tauri/src/sync2/snapshot.rs @@ -3,7 +3,7 @@ use std::collections::BTreeMap; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use sqlx::SqlitePool; use super::batches::batch_content; @@ -32,11 +32,21 @@ const BLOB_CONTENT_LIMIT: usize = 2 * 1024 * 1024; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct Part { + #[serde(deserialize_with = "blob_name")] pub blob: String, pub collection: String, pub size: u64, } +/// A blob's name is the hex of its SHA-256 (spec/store.md, Snapshot blobs). It becomes a store path. +fn blob_name<'de, D: Deserializer<'de>>(deserializer: D) -> std::result::Result { + let name = String::deserialize(deserializer)?; + let valid = name.len() == 64 && name.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'f')); + valid + .then_some(name) + .ok_or_else(|| serde::de::Error::custom("not a blob name")) +} + /// The last object of a log that a snapshot includes, so reading can go on from it. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(deny_unknown_fields)] @@ -255,6 +265,26 @@ mod tests { assert!(manifest(1 << 53).is_err()); } + #[test] + fn a_manifest_names_blobs_only_by_their_hash() { + let manifest = |blob: &str| { + serde_json::from_str::(&format!( + r#"{{"head":"","link":null,"parts":[{{"blob":"{blob}","collection":"c","size":1}}],"vv":{{}}}}"# + )) + }; + assert!(manifest(&"a1".repeat(32)).is_ok()); + for blob in [ + "", + "ab", + "../x", + &"A1".repeat(32), + &"g1".repeat(32), + &"a1".repeat(33), + ] { + assert!(manifest(blob).is_err(), "{blob}"); + } + } + #[test] fn records_are_split_below_the_blob_limit() { let record = vec![b'x'; BLOB_CONTENT_LIMIT / 3]; diff --git a/src-tauri/src/sync2/webdav/client.rs b/src-tauri/src/sync2/webdav/client.rs index a3eb2ce9..1bfecb1e 100644 --- a/src-tauri/src/sync2/webdav/client.rs +++ b/src-tauri/src/sync2/webdav/client.rs @@ -53,6 +53,9 @@ impl WebDav { if self.refused.load(Ordering::SeqCst) { return Err(StoreError::Unauthorized); } + if !inside_folder(path) { + return Err(StoreError::Malformed); + } let url = self.base.join(path).map_err(|_| StoreError::Malformed)?; Ok(self .client @@ -93,6 +96,15 @@ impl WebDav { } } +/// Every path this client builds is plain names under the group's folder, so a URL can't resolve +/// anywhere else on the account. +fn inside_folder(path: &str) -> bool { + let plain = path + .bytes() + .all(|b| b.is_ascii_alphanumeric() || matches!(b, b'.' | b'-' | b'_' | b'/')); + plain && !path.starts_with('/') && !path.split('/').any(|part| part == "." || part == "..") +} + fn refused(response: &Response) -> StoreError { StoreError::Refused { status: response.status().as_u16(), diff --git a/src-tauri/tests/sync2_webdav.rs b/src-tauri/tests/sync2_webdav.rs index 1ebd6347..cc67507e 100644 --- a/src-tauri/tests/sync2_webdav.rs +++ b/src-tauri/tests/sync2_webdav.rs @@ -380,6 +380,48 @@ async fn a_group_in_a_folder_is_restored_with_its_recovery_words() { assert!(matches!(used, Err(EngineError::RecoveryUsed))); } +#[tokio::test] +async fn a_client_reaches_only_paths_inside_its_folder() { + let (root, _) = webdav_server().await; + let account = webdav(&root.replace("flow-sync/", ""), PASSWORD).await; + account + .put("other/notes.txt", b"mine".to_vec()) + .await + .unwrap(); + let client = webdav(&root, PASSWORD).await; + client.put("x/inside.b", b"ours".to_vec()).await.unwrap(); + + for path in [ + "../other/notes.txt", + "x/../../other/notes.txt", + "%2e%2e/other/notes.txt", + "..%2fother/notes.txt", + "/other/notes.txt", + "../other/notes.txt#", + "x/inside.b#", + "x/inside.b?y", + "x\\..\\..\\other", + ] { + assert!( + matches!(client.get(path).await, Err(StoreError::Malformed)), + "{path}" + ); + assert!( + matches!(client.delete(path).await, Err(StoreError::Malformed)), + "{path}" + ); + assert!( + matches!(client.put(path, vec![1]).await, Err(StoreError::Malformed)), + "{path}" + ); + } + assert_eq!( + account.get("other/notes.txt").await.unwrap().unwrap(), + b"mine" + ); + assert_eq!(client.get("x/inside.b").await.unwrap().unwrap(), b"ours"); +} + #[tokio::test] async fn a_refused_password_stops_every_later_request() { let (url, requests) = webdav_server().await;