From 895e611c155da47a8bfa82149360a1d07e0e1f58 Mon Sep 17 00:00:00 2001 From: Buckram Date: Thu, 22 Feb 2024 16:48:39 +0200 Subject: [PATCH 1/8] symlinks does not work in debug mode --- examples/public/symlinks/main.js | 1 + tests/include_exclude.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 120000 examples/public/symlinks/main.js diff --git a/examples/public/symlinks/main.js b/examples/public/symlinks/main.js new file mode 120000 index 0000000..05fdb15 --- /dev/null +++ b/examples/public/symlinks/main.js @@ -0,0 +1 @@ +../main.js \ No newline at end of file diff --git a/tests/include_exclude.rs b/tests/include_exclude.rs index e49ca89..b56d66b 100644 --- a/tests/include_exclude.rs +++ b/tests/include_exclude.rs @@ -52,3 +52,15 @@ fn exclude_has_higher_priority() { assert!(ExcludePriorityAssets::get("images/llama.png").is_some(), "llama.png should exist"); assert_eq!(ExcludePriorityAssets::iter().count(), 2); } + +#[derive(RustEmbed)] +#[folder = "examples/public/symlinks"] +#[include = "main.js"] +struct IncludeSymlink; + +#[test] +fn include_symlink() { + assert_eq!(IncludeSymlink::iter().count(), 1); + assert_eq!(IncludeSymlink::iter().next(), Some(std::borrow::Cow::Borrowed("main.js"))); + assert!(IncludeSymlink::get("main.js").is_some()) +} \ No newline at end of file From f09962831bee87c5d464811860704193fde56c88 Mon Sep 17 00:00:00 2001 From: Buckram Date: Thu, 22 Feb 2024 16:51:28 +0200 Subject: [PATCH 2/8] fmt --- tests/include_exclude.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/include_exclude.rs b/tests/include_exclude.rs index b56d66b..e49c667 100644 --- a/tests/include_exclude.rs +++ b/tests/include_exclude.rs @@ -63,4 +63,4 @@ fn include_symlink() { assert_eq!(IncludeSymlink::iter().count(), 1); assert_eq!(IncludeSymlink::iter().next(), Some(std::borrow::Cow::Borrowed("main.js"))); assert!(IncludeSymlink::get("main.js").is_some()) -} \ No newline at end of file +} From 47e1f89c334d11bd14c8b8a29a237e9072d0e7db Mon Sep 17 00:00:00 2001 From: Buckram Date: Sat, 24 Feb 2024 14:55:30 +0200 Subject: [PATCH 3/8] Fixed symlinks in debug mode --- impl/src/lib.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/impl/src/lib.rs b/impl/src/lib.rs index 114456b..549df96 100644 --- a/impl/src/lib.rs +++ b/impl/src/lib.rs @@ -141,7 +141,16 @@ fn dynamic(ident: &syn::Ident, folder_path: String, prefix: Option<&str>, includ let canonical_file_path = file_path.canonicalize().ok()?; if !canonical_file_path.starts_with(#canonical_folder_path) { // Tried to request a path that is not in the embedded folder - return ::std::option::Option::None; + + // Should be allowed only if it was a symlink + // TODO: Currently it allows "path_traversal_attack" for the symlink files + // For it to be working properly we need to get absolute path first + // and check that instead if it starts with `canonical_folder_path` + // https://doc.rust-lang.org/std/path/fn.absolute.html (currently nightly) + let metadata = ::std::fs::metadata(file_path.as_path()).ok()?; + if !metadata.is_symlink() { + return ::std::option::Option::None; + } } if rust_embed::utils::is_path_included(&rel_file_path, INCLUDES, EXCLUDES) { From 249ee3c8f1dc36f74f5523eb54e38fbafeb395ed Mon Sep 17 00:00:00 2001 From: Buckram Date: Sat, 24 Feb 2024 14:55:50 +0200 Subject: [PATCH 4/8] tests fixes --- tests/include_exclude.rs | 6 ++++-- tests/interpolated_path.rs | 4 ++-- tests/lib.rs | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/include_exclude.rs b/tests/include_exclude.rs index e49c667..a0dd99d 100644 --- a/tests/include_exclude.rs +++ b/tests/include_exclude.rs @@ -9,7 +9,8 @@ fn get_works() { assert!(AllAssets::get("index.html").is_some(), "index.html should exist"); assert!(AllAssets::get("gg.html").is_none(), "gg.html should not exist"); assert!(AllAssets::get("images/llama.png").is_some(), "llama.png should exist"); - assert_eq!(AllAssets::iter().count(), 6); + assert!(AllAssets::get("symlinks/main.js").is_some(), "main.js should exist"); + assert_eq!(AllAssets::iter().count(), 7); } #[derive(RustEmbed)] @@ -36,8 +37,9 @@ struct ExcludeSomeAssets; fn excluding_some_assets_works() { assert!(ExcludeSomeAssets::get("index.html").is_none(), "index.html should not exist"); assert!(ExcludeSomeAssets::get("main.js").is_some(), "main.js should exist"); + assert!(ExcludeSomeAssets::get("symlinks/main.js").is_some(), "main.js symlink should exist"); assert!(ExcludeSomeAssets::get("images/llama.png").is_none(), "llama.png should not exist"); - assert_eq!(ExcludeSomeAssets::iter().count(), 2); + assert_eq!(ExcludeSomeAssets::iter().count(), 3); } #[derive(RustEmbed)] diff --git a/tests/interpolated_path.rs b/tests/interpolated_path.rs index 0df2366..e446eef 100644 --- a/tests/interpolated_path.rs +++ b/tests/interpolated_path.rs @@ -19,7 +19,7 @@ fn iter_works() { assert!(Asset::get(file.as_ref()).is_some()); num_files += 1; } - assert_eq!(num_files, 6); + assert_eq!(num_files, 7); } #[test] @@ -32,6 +32,6 @@ fn trait_works_generic_helper() { assert!(E::get(file.as_ref()).is_some()); num_files += 1; } - assert_eq!(num_files, 6); + assert_eq!(num_files, 7); assert!(E::get("gg.html").is_none(), "gg.html should not exist"); } diff --git a/tests/lib.rs b/tests/lib.rs index a4636aa..b3d6433 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -28,7 +28,7 @@ fn iter_works() { assert!(Asset::get(file.as_ref()).is_some()); num_files += 1; } - assert_eq!(num_files, 6); + assert_eq!(num_files, 7); } #[test] @@ -41,6 +41,6 @@ fn trait_works_generic_helper() { assert!(E::get(file.as_ref()).is_some()); num_files += 1; } - assert_eq!(num_files, 6); + assert_eq!(num_files, 7); assert!(E::get("gg.html").is_none(), "gg.html should not exist"); } From 42bb1fee430a99060d64cb1bea09e98a2c5a2347 Mon Sep 17 00:00:00 2001 From: Buckram Date: Sat, 24 Feb 2024 14:58:11 +0200 Subject: [PATCH 5/8] format --- impl/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impl/src/lib.rs b/impl/src/lib.rs index 549df96..f417733 100644 --- a/impl/src/lib.rs +++ b/impl/src/lib.rs @@ -144,7 +144,7 @@ fn dynamic(ident: &syn::Ident, folder_path: String, prefix: Option<&str>, includ // Should be allowed only if it was a symlink // TODO: Currently it allows "path_traversal_attack" for the symlink files - // For it to be working properly we need to get absolute path first + // For it to be working properly we need to get absolute path first // and check that instead if it starts with `canonical_folder_path` // https://doc.rust-lang.org/std/path/fn.absolute.html (currently nightly) let metadata = ::std::fs::metadata(file_path.as_path()).ok()?; From 639ec4dc403aa5e0889c0eee088453bed2caf386 Mon Sep 17 00:00:00 2001 From: Buckram Date: Sat, 24 Feb 2024 15:06:25 +0200 Subject: [PATCH 6/8] found better function --- impl/src/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/impl/src/lib.rs b/impl/src/lib.rs index f417733..d665e6d 100644 --- a/impl/src/lib.rs +++ b/impl/src/lib.rs @@ -143,11 +143,7 @@ fn dynamic(ident: &syn::Ident, folder_path: String, prefix: Option<&str>, includ // Tried to request a path that is not in the embedded folder // Should be allowed only if it was a symlink - // TODO: Currently it allows "path_traversal_attack" for the symlink files - // For it to be working properly we need to get absolute path first - // and check that instead if it starts with `canonical_folder_path` - // https://doc.rust-lang.org/std/path/fn.absolute.html (currently nightly) - let metadata = ::std::fs::metadata(file_path.as_path()).ok()?; + let metadata = ::std::fs::symlink_metadata(file_path.as_path()).ok()?; if !metadata.is_symlink() { return ::std::option::Option::None; } From e7c5447d786d69e7ba70d1dbf1d1ca3970bb68a0 Mon Sep 17 00:00:00 2001 From: Buckram Date: Sat, 24 Feb 2024 15:29:55 +0200 Subject: [PATCH 7/8] add todo --- impl/src/lib.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/impl/src/lib.rs b/impl/src/lib.rs index d665e6d..cedb14b 100644 --- a/impl/src/lib.rs +++ b/impl/src/lib.rs @@ -142,6 +142,10 @@ fn dynamic(ident: &syn::Ident, folder_path: String, prefix: Option<&str>, includ if !canonical_file_path.starts_with(#canonical_folder_path) { // Tried to request a path that is not in the embedded folder + // TODO: Currently it allows "path_traversal_attack" for the symlink files + // For it to be working properly we need to get absolute path first + // and check that instead if it starts with `canonical_folder_path` + // https://doc.rust-lang.org/std/path/fn.absolute.html (currently nightly) // Should be allowed only if it was a symlink let metadata = ::std::fs::symlink_metadata(file_path.as_path()).ok()?; if !metadata.is_symlink() { From 23c8cbf2c6a4574504f17062b6cf4304765791ad Mon Sep 17 00:00:00 2001 From: Buckram Date: Sat, 24 Feb 2024 15:37:36 +0200 Subject: [PATCH 8/8] add symlink traversal attack test --- tests/path_traversal_attack.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/path_traversal_attack.rs b/tests/path_traversal_attack.rs index 43e4e8b..63e9bee 100644 --- a/tests/path_traversal_attack.rs +++ b/tests/path_traversal_attack.rs @@ -11,3 +11,17 @@ struct Assets; fn path_traversal_attack_fails() { assert!(Assets::get("../basic.rs").is_none()); } + +#[derive(RustEmbed)] +#[folder = "examples/axum-spa/"] +struct AxumAssets; + +// TODO: +/// Prevent attempts to access symlinks outside of the embedded folder. +/// This is mainly a concern when running in debug mode, since that loads from +/// the file system at runtime. +#[test] +#[ignore = "see https://github.com/pyrossh/rust-embed/pull/235"] +fn path_traversal_attack_symlink_fails() { + assert!(Assets::get("../public/symlinks/main.js").is_none()); +}