AssetServer out of bounds protection #18088
Conversation
alice-i-cecile
added
C-Bug
|
I added more tests. Now a CI/build fails on github, but succeeds on my machine. Idk what to do from here |
| 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> { |
There was a problem hiding this comment.
I don't like the duplication of all of these functions. In almost all cases you'd set the mode globally and never need the other variant I think? And if we really want a way to opt out of the sandbox can't we just make load_with_meta_transform public instead of creating every combination of how it can be called as separate methods?
There was a problem hiding this comment.
If you have a scripting system, you would give that system access to AssetServer::load, and then use AssetServer::load_override. If UnapprovedPathMode (previously OutOfBoundsMode) is Deny, this would let the developers load whatever files, while preventing mods from doing the same.
We could get rid of all _override functions and expose load_with_meta_transform, but that would make using this security feature more confusing
Co-authored-by: Kristoffer Søholm <k.soeholm@gmail.com>
|
Now denied asset loads return |
crates/bevy_asset/src/lib.rs
Outdated
| @@ -1902,7 +1902,7 @@ mod tests { | |||
| dir.insert_asset_text(Path::new(a_path), a_ron); | |||
|
|
|||
| let mut app = App::new(); | |||
| let (gated_memory_reader, gate_opener) = GatedReader::new(MemoryAssetReader { root: dir }); | |||
| let (gated_memory_reader, _gate_opener) = GatedReader::new(MemoryAssetReader { root: dir }); | |||
There was a problem hiding this comment.
This is still not quite right. Just remove the GatedReader::new entirely, and I think it should work.
Addresses Issue #18073