AssetServer out of bounds protection (#18088)

Addresses Issue #18073

---------

Co-authored-by: Threadzless <threadzless@gmail.com>
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>

crates/bevy_asset/src/lib.rs

@@ -258,6 +258,33 @@ pub struct AssetPlugin {
     pub mode: AssetMode,
     /// How/If asset meta files should be checked.
     pub meta_check: AssetMetaCheck,
+    /// How to handle load requests of files that are outside the approved directories.
+    ///
+    /// Approved folders are [`AssetPlugin::file_path`] and the folder of each
+    /// [`AssetSource`](io::AssetSource). Subfolders within these folders are also valid.
+    pub unapproved_path_mode: UnapprovedPathMode,
+}
+
+/// Determines how to react to attempts to load assets not inside the approved folders.
+///
+/// Approved folders are [`AssetPlugin::file_path`] and the folder of each
+/// [`AssetSource`](io::AssetSource). Subfolders within these folders are also valid.
+///
+/// It is strongly discouraged to use [`Allow`](UnapprovedPathMode::Allow) if your
+/// app will include scripts or modding support, as it could allow allow arbitrary file
+/// access for malicious code.
+///
+/// See [`AssetPath::is_unapproved`](crate::AssetPath::is_unapproved)
+#[derive(Clone, Default)]
+pub enum UnapprovedPathMode {
+    /// Unapproved asset loading is allowed. This is strongly discouraged.
+    Allow,
+    /// Fails to load any asset that is is unapproved, unless an override method is used, like
+    /// [`AssetServer::load_override`].
+    Deny,
+    /// Fails to load any asset that is is unapproved.
+    #[default]
+    Forbid,
 }
 
 /// Controls whether or not assets are pre-processed before being loaded.
@@ -311,6 +338,7 @@ impl Default for AssetPlugin {
             processed_file_path: Self::DEFAULT_PROCESSED_FILE_PATH.to_string(),
             watch_for_changes_override: None,
             meta_check: AssetMetaCheck::default(),
+            unapproved_path_mode: UnapprovedPathMode::default(),
         }
     }
 }
@@ -351,6 +379,7 @@ impl Plugin for AssetPlugin {
                         AssetServerMode::Unprocessed,
                         self.meta_check.clone(),
                         watch,
+                        self.unapproved_path_mode.clone(),
                     ));
                 }
                 AssetMode::Processed => {
@@ -367,6 +396,7 @@ impl Plugin for AssetPlugin {
                             AssetServerMode::Processed,
                             AssetMetaCheck::Always,
                             watch,
+                            self.unapproved_path_mode.clone(),
                         ))
                         .insert_resource(processor)
                         .add_systems(bevy_app::Startup, AssetProcessor::start);
@@ -380,6 +410,7 @@ impl Plugin for AssetPlugin {
                             AssetServerMode::Processed,
                             AssetMetaCheck::Always,
                             watch,
+                            self.unapproved_path_mode.clone(),
                         ));
                     }
                 }
@@ -639,7 +670,7 @@ mod tests {
         },
         loader::{AssetLoader, LoadContext},
         Asset, AssetApp, AssetEvent, AssetId, AssetLoadError, AssetLoadFailedEvent, AssetPath,
-        AssetPlugin, AssetServer, Assets, DuplicateLabelAssetError, LoadState,
+        AssetPlugin, AssetServer, Assets, DuplicateLabelAssetError, LoadState, UnapprovedPathMode,
     };
     use alloc::{
         boxed::Box,
@@ -1933,4 +1964,92 @@ mod tests {
 
     #[derive(Asset, TypePath)]
     pub struct TupleTestAsset(#[dependency] Handle<TestAsset>);
+
+    fn unapproved_path_setup(mode: UnapprovedPathMode) -> App {
+        let dir = Dir::default();
+        let a_path = "../a.cool.ron";
+        let a_ron = r#"
+(
+    text: "a",
+    dependencies: [],
+    embedded_dependencies: [],
+    sub_texts: [],
+)"#;
+
+        dir.insert_asset_text(Path::new(a_path), a_ron);
+
+        let mut app = App::new();
+        let memory_reader = MemoryAssetReader { root: dir };
+        app.register_asset_source(
+            AssetSourceId::Default,
+            AssetSource::build().with_reader(move || Box::new(memory_reader.clone())),
+        )
+        .add_plugins((
+            TaskPoolPlugin::default(),
+            LogPlugin::default(),
+            AssetPlugin {
+                unapproved_path_mode: mode,
+                ..Default::default()
+            },
+        ));
+        app.init_asset::<CoolText>();
+
+        app
+    }
+
+    fn load_a_asset(assets: Res<AssetServer>) {
+        let a = assets.load::<CoolText>("../a.cool.ron");
+        if a == Handle::default() {
+            panic!()
+        }
+    }
+
+    fn load_a_asset_override(assets: Res<AssetServer>) {
+        let a = assets.load_override::<CoolText>("../a.cool.ron");
+        if a == Handle::default() {
+            panic!()
+        }
+    }
+
+    #[test]
+    #[should_panic]
+    fn unapproved_path_forbid_should_panic() {
+        let mut app = unapproved_path_setup(UnapprovedPathMode::Forbid);
+
+        fn uses_assets(_asset: ResMut<Assets<CoolText>>) {}
+        app.add_systems(Update, (uses_assets, load_a_asset_override));
+
+        app.world_mut().run_schedule(Update);
+    }
+
+    #[test]
+    #[should_panic]
+    fn unapproved_path_deny_should_panic() {
+        let mut app = unapproved_path_setup(UnapprovedPathMode::Deny);
+
+        fn uses_assets(_asset: ResMut<Assets<CoolText>>) {}
+        app.add_systems(Update, (uses_assets, load_a_asset));
+
+        app.world_mut().run_schedule(Update);
+    }
+
+    #[test]
+    fn unapproved_path_deny_should_finish() {
+        let mut app = unapproved_path_setup(UnapprovedPathMode::Deny);
+
+        fn uses_assets(_asset: ResMut<Assets<CoolText>>) {}
+        app.add_systems(Update, (uses_assets, load_a_asset_override));
+
+        app.world_mut().run_schedule(Update);
+    }
+
+    #[test]
+    fn unapproved_path_allow_should_finish() {
+        let mut app = unapproved_path_setup(UnapprovedPathMode::Allow);
+
+        fn uses_assets(_asset: ResMut<Assets<CoolText>>) {}
+        app.add_systems(Update, (uses_assets, load_a_asset));
+
+        app.world_mut().run_schedule(Update);
+    }
 }

crates/bevy_asset/src/loader_builders.rs

@@ -305,9 +305,12 @@ impl NestedLoader<'_, '_, StaticTyped, Deferred> {
     pub fn load<'c, A: Asset>(self, path: impl Into<AssetPath<'c>>) -> Handle<A> {
         let path = path.into().to_owned();
         let handle = if self.load_context.should_load_dependencies {
-            self.load_context
+            self.load_context.asset_server.load_with_meta_transform(
-                .asset_server
+                path,
-                .load_with_meta_transform(path, self.meta_transform, ())
+                self.meta_transform,
+                (),
+                true,
+            )
         } else {
             self.load_context
                 .asset_server

crates/bevy_asset/src/path.rs

@@ -478,6 +478,51 @@ impl<'a> AssetPath<'a> {
             }
         })
     }
+
+    /// Returns `true` if this [`AssetPath`] points to a file that is
+    /// outside of it's [`AssetSource`](crate::io::AssetSource) folder.
+    ///
+    /// ## Example
+    /// ```
+    /// # use bevy_asset::AssetPath;
+    /// // Inside the default AssetSource.
+    /// let path = AssetPath::parse("thingy.png");
+    /// assert!( ! path.is_unapproved());
+    /// let path = AssetPath::parse("gui/thingy.png");
+    /// assert!( ! path.is_unapproved());
+    ///
+    /// // Inside a different AssetSource.
+    /// let path = AssetPath::parse("embedded://thingy.png");
+    /// assert!( ! path.is_unapproved());
+    ///
+    /// // Exits the `AssetSource`s directory.
+    /// let path = AssetPath::parse("../thingy.png");
+    /// assert!(path.is_unapproved());
+    /// let path = AssetPath::parse("folder/../../thingy.png");
+    /// assert!(path.is_unapproved());
+    ///
+    /// // This references the linux root directory.
+    /// let path = AssetPath::parse("/home/thingy.png");
+    /// assert!(path.is_unapproved());
+    /// ```
+    pub fn is_unapproved(&self) -> bool {
+        use std::path::Component;
+        let mut simplified = PathBuf::new();
+        for component in self.path.components() {
+            match component {
+                Component::Prefix(_) | Component::RootDir => return true,
+                Component::CurDir => {}
+                Component::ParentDir => {
+                    if !simplified.pop() {
+                        return true;
+                    }
+                }
+                Component::Normal(os_str) => simplified.push(os_str),
+            }
+        }
+
+        false
+    }
 }
 
 impl AssetPath<'static> {

crates/bevy_asset/src/processor/mod.rs

@@ -54,7 +54,7 @@ use crate::{
         AssetMetaDyn, AssetMetaMinimal, ProcessedInfo, ProcessedInfoMinimal,
     },
     AssetLoadError, AssetMetaCheck, AssetPath, AssetServer, AssetServerMode, DeserializeMetaError,
-    MissingAssetLoaderForExtensionError, WriteDefaultMetaError,
+    MissingAssetLoaderForExtensionError, UnapprovedPathMode, WriteDefaultMetaError,
 };
 use alloc::{borrow::ToOwned, boxed::Box, collections::VecDeque, sync::Arc, vec, vec::Vec};
 use bevy_ecs::prelude::*;
@@ -122,6 +122,7 @@ impl AssetProcessor {
             AssetServerMode::Processed,
             AssetMetaCheck::Always,
             false,
+            UnapprovedPathMode::default(),
         );
         Self { server, data }
     }

crates/bevy_asset/src/server/mod.rs

@@ -16,7 +16,7 @@ use crate::{
     path::AssetPath,
     Asset, AssetEvent, AssetHandleProvider, AssetId, AssetLoadFailedEvent, AssetMetaCheck, Assets,
     CompleteErasedLoadedAsset, DeserializeMetaError, ErasedLoadedAsset, Handle, LoadedUntypedAsset,
-    UntypedAssetId, UntypedAssetLoadFailedEvent, UntypedHandle,
+    UnapprovedPathMode, UntypedAssetId, UntypedAssetLoadFailedEvent, UntypedHandle,
 };
 use alloc::{borrow::ToOwned, boxed::Box, vec, vec::Vec};
 use alloc::{
@@ -68,6 +68,7 @@ pub(crate) struct AssetServerData {
     sources: AssetSources,
     mode: AssetServerMode,
     meta_check: AssetMetaCheck,
+    unapproved_path_mode: UnapprovedPathMode,
 }
 
 /// The "asset mode" the server is currently in.
@@ -82,13 +83,19 @@ pub enum AssetServerMode {
 impl AssetServer {
     /// Create a new instance of [`AssetServer`]. If `watch_for_changes` is true, the [`AssetReader`](crate::io::AssetReader) storage will watch for changes to
     /// asset sources and hot-reload them.
-    pub fn new(sources: AssetSources, mode: AssetServerMode, watching_for_changes: bool) -> Self {
+    pub fn new(
+        sources: AssetSources,
+        mode: AssetServerMode,
+        watching_for_changes: bool,
+        unapproved_path_mode: UnapprovedPathMode,
+    ) -> Self {
         Self::new_with_loaders(
             sources,
             Default::default(),
             mode,
             AssetMetaCheck::Always,
             watching_for_changes,
+            unapproved_path_mode,
         )
     }
 
@@ -99,6 +106,7 @@ impl AssetServer {
         mode: AssetServerMode,
         meta_check: AssetMetaCheck,
         watching_for_changes: bool,
+        unapproved_path_mode: UnapprovedPathMode,
     ) -> Self {
         Self::new_with_loaders(
             sources,
@@ -106,6 +114,7 @@ impl AssetServer {
             mode,
             meta_check,
             watching_for_changes,
+            unapproved_path_mode,
         )
     }
 
@@ -115,6 +124,7 @@ impl AssetServer {
         mode: AssetServerMode,
         meta_check: AssetMetaCheck,
         watching_for_changes: bool,
+        unapproved_path_mode: UnapprovedPathMode,
     ) -> Self {
         let (asset_event_sender, asset_event_receiver) = crossbeam_channel::unbounded();
         let mut infos = AssetInfos::default();
@@ -128,6 +138,7 @@ impl AssetServer {
                 asset_event_receiver,
                 loaders,
                 infos: RwLock::new(infos),
+                unapproved_path_mode,
             }),
         }
     }
@@ -311,7 +322,16 @@ impl AssetServer {
     /// The asset load will fail and an error will be printed to the logs if the asset stored at `path` is not of type `A`.
     #[must_use = "not using the returned strong handle may result in the unexpected release of the asset"]
     pub fn load<'a, A: Asset>(&self, path: impl Into<AssetPath<'a>>) -> Handle<A> {
-        self.load_with_meta_transform(path, None, ())
+        self.load_with_meta_transform(path, None, (), false)
+    }
+
+    /// Same as [`load`](AssetServer::load), but you can load assets from unaproved paths
+    /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode)
+    /// is [`Deny`](UnapprovedPathMode::Deny).
+    ///
+    /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`]
+    pub fn load_override<'a, A: Asset>(&self, path: impl Into<AssetPath<'a>>) -> Handle<A> {
+        self.load_with_meta_transform(path, None, (), true)
     }
 
     /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item.
@@ -335,7 +355,20 @@ impl AssetServer {
         path: impl Into<AssetPath<'a>>,
         guard: G,
     ) -> Handle<A> {
-        self.load_with_meta_transform(path, None, guard)
+        self.load_with_meta_transform(path, None, guard, false)
+    }
+
+    /// Same as [`load`](AssetServer::load_acquire), but you can load assets from unaproved paths
+    /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode)
+    /// is [`Deny`](UnapprovedPathMode::Deny).
+    ///
+    /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`]
+    pub fn load_acquire_override<'a, A: Asset, G: Send + Sync + 'static>(
+        &self,
+        path: impl Into<AssetPath<'a>>,
+        guard: G,
+    ) -> Handle<A> {
+        self.load_with_meta_transform(path, None, guard, true)
     }
 
     /// Begins loading an [`Asset`] of type `A` stored at `path`. The given `settings` function will override the asset's
@@ -347,7 +380,30 @@ impl AssetServer {
         path: impl Into<AssetPath<'a>>,
         settings: impl Fn(&mut S) + Send + Sync + 'static,
     ) -> Handle<A> {
-        self.load_with_meta_transform(path, Some(loader_settings_meta_transform(settings)), ())
+        self.load_with_meta_transform(
+            path,
+            Some(loader_settings_meta_transform(settings)),
+            (),
+            false,
+        )
+    }
+
+    /// Same as [`load`](AssetServer::load_with_settings), but you can load assets from unaproved paths
+    /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode)
+    /// is [`Deny`](UnapprovedPathMode::Deny).
+    ///
+    /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`]
+    pub fn load_with_settings_override<'a, A: Asset, S: Settings>(
+        &self,
+        path: impl Into<AssetPath<'a>>,
+        settings: impl Fn(&mut S) + Send + Sync + 'static,
+    ) -> Handle<A> {
+        self.load_with_meta_transform(
+            path,
+            Some(loader_settings_meta_transform(settings)),
+            (),
+            true,
+        )
     }
 
     /// Begins loading an [`Asset`] of type `A` stored at `path` while holding a guard item.
@@ -366,7 +422,36 @@ impl AssetServer {
         settings: impl Fn(&mut S) + Send + Sync + 'static,
         guard: G,
     ) -> Handle<A> {
-        self.load_with_meta_transform(path, Some(loader_settings_meta_transform(settings)), guard)
+        self.load_with_meta_transform(
+            path,
+            Some(loader_settings_meta_transform(settings)),
+            guard,
+            false,
+        )
+    }
+
+    /// Same as [`load`](AssetServer::load_acquire_with_settings), but you can load assets from unaproved paths
+    /// if [`AssetPlugin::unapproved_path_mode`](super::AssetPlugin::unapproved_path_mode)
+    /// is [`Deny`](UnapprovedPathMode::Deny).
+    ///
+    /// See [`UnapprovedPathMode`] and [`AssetPath::is_unapproved`]
+    pub fn load_acquire_with_settings_override<
+        'a,
+        A: Asset,
+        S: Settings,
+        G: Send + Sync + 'static,
+    >(
+        &self,
+        path: impl Into<AssetPath<'a>>,
+        settings: impl Fn(&mut S) + Send + Sync + 'static,
+        guard: G,
+    ) -> Handle<A> {
+        self.load_with_meta_transform(
+            path,
+            Some(loader_settings_meta_transform(settings)),
+            guard,
+            true,
+        )
     }
 
     pub(crate) fn load_with_meta_transform<'a, A: Asset, G: Send + Sync + 'static>(
@@ -374,8 +459,20 @@ impl AssetServer {
         path: impl Into<AssetPath<'a>>,
         meta_transform: Option<MetaTransform>,
         guard: G,
+        override_unapproved: bool,
     ) -> Handle<A> {
         let path = path.into().into_owned();
+
+        if path.is_unapproved() {
+            match (&self.data.unapproved_path_mode, override_unapproved) {
+                (UnapprovedPathMode::Allow, _) | (UnapprovedPathMode::Deny, true) => {}
+                (UnapprovedPathMode::Deny, false) | (UnapprovedPathMode::Forbid, _) => {
+                    error!("Asset path {path} is unapproved. See UnapprovedPathMode for details.");
+                    return Handle::default();
+                }
+            }
+        }
+
         let mut infos = self.data.infos.write();
         let (handle, should_load) = infos.get_or_create_path_handle::<A>(
             path.clone(),