From c37a447fabce4f2342a0fa82910f706691d9e146 Mon Sep 17 00:00:00 2001 From: "K. S. Ernest (iFire) Lee" Date: Fri, 22 Nov 2024 23:52:49 -0800 Subject: [PATCH] Add external path and type whitelist to ResourceLoader --- core/core_bind.cpp | 14 ++ core/core_bind.h | 2 + core/io/resource_format_binary.cpp | 64 ++++++- core/io/resource_format_binary.h | 5 + core/io/resource_importer.cpp | 2 +- core/io/resource_loader.cpp | 70 ++++++- core/io/resource_loader.h | 12 +- doc/classes/ResourceLoader.xml | 27 +++ scene/resources/resource_format_text.cpp | 127 +++++++++---- scene/resources/resource_format_text.h | 5 + .../authorized_resource.res | Bin 0 -> 397 bytes .../authorized_resource.tres | 11 ++ .../trojan_resource.res | Bin 0 -> 398 bytes .../trojan_resource.tres | 12 ++ .../authorized_resource.res | Bin 0 -> 410 bytes .../authorized_resource.tres | 12 ++ .../trojan_resource.res | Bin 0 -> 411 bytes .../trojan_resource.tres | 12 ++ tests/data/null_path/.gitkeep | 0 tests/scene/test_resource_loader.h | 173 ++++++++++++++++++ tests/test_main.cpp | 1 + 21 files changed, 500 insertions(+), 49 deletions(-) create mode 100644 tests/data/load_resource_golden_path/authorized_resource.res create mode 100644 tests/data/load_resource_golden_path/authorized_resource.tres create mode 100644 tests/data/load_resource_malicious_path/trojan_resource.res create mode 100644 tests/data/load_resource_malicious_path/trojan_resource.tres create mode 100644 tests/data/load_resource_whitelisted_golden_path/authorized_resource.res create mode 100644 tests/data/load_resource_whitelisted_golden_path/authorized_resource.tres create mode 100644 tests/data/load_resource_whitelisted_malicious_path/trojan_resource.res create mode 100644 tests/data/load_resource_whitelisted_malicious_path/trojan_resource.tres create mode 100644 tests/data/null_path/.gitkeep create mode 100644 tests/scene/test_resource_loader.h diff --git a/core/core_bind.cpp b/core/core_bind.cpp index acd589783ece..282f7364d231 100644 --- a/core/core_bind.cpp +++ b/core/core_bind.cpp @@ -52,6 +52,10 @@ Error ResourceLoader::load_threaded_request(const String &p_path, const String & return ::ResourceLoader::load_threaded_request(p_path, p_type_hint, p_use_sub_threads, ResourceFormatLoader::CacheMode(p_cache_mode)); } +Error ResourceLoader::load_threaded_request_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint, bool p_use_sub_threads, CacheMode p_cache_mode) { + return ::ResourceLoader::load_threaded_request_whitelisted(p_path, p_external_path_whitelist, p_type_whitelist, p_type_hint, p_use_sub_threads, ResourceFormatLoader::CacheMode(p_cache_mode)); +} + ResourceLoader::ThreadLoadStatus ResourceLoader::load_threaded_get_status(const String &p_path, Array r_progress) { float progress = 0; ::ResourceLoader::ThreadLoadStatus tls = ::ResourceLoader::load_threaded_get_status(p_path, &progress); @@ -77,6 +81,14 @@ Ref ResourceLoader::load(const String &p_path, const String &p_type_hi return ret; } +Ref ResourceLoader::load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint, CacheMode p_cache_mode) { + Error err = OK; + Ref ret = ::ResourceLoader::load_whitelisted(p_path, p_external_path_whitelist, p_type_whitelist, p_type_hint, ResourceFormatLoader::CacheMode(p_cache_mode), &err); + + ERR_FAIL_COND_V_MSG(err != OK, ret, "Error loading resource: '" + p_path + "'."); + return ret; +} + Vector ResourceLoader::get_recognized_extensions_for_type(const String &p_type) { List exts; ::ResourceLoader::get_recognized_extensions_for_type(p_type, &exts); @@ -136,10 +148,12 @@ Vector ResourceLoader::list_directory(const String &p_directory) { void ResourceLoader::_bind_methods() { ClassDB::bind_method(D_METHOD("load_threaded_request", "path", "type_hint", "use_sub_threads", "cache_mode"), &ResourceLoader::load_threaded_request, DEFVAL(""), DEFVAL(false), DEFVAL(CACHE_MODE_REUSE)); + ClassDB::bind_method(D_METHOD("load_threaded_request_whitelisted", "path", "external_path_whitelist", "type_whitelist", "type_hint", "use_sub_threads", "cache_mode"), &ResourceLoader::load_threaded_request_whitelisted, DEFVAL(""), DEFVAL(false), DEFVAL(CACHE_MODE_REUSE)); ClassDB::bind_method(D_METHOD("load_threaded_get_status", "path", "progress"), &ResourceLoader::load_threaded_get_status, DEFVAL_ARRAY); ClassDB::bind_method(D_METHOD("load_threaded_get", "path"), &ResourceLoader::load_threaded_get); ClassDB::bind_method(D_METHOD("load", "path", "type_hint", "cache_mode"), &ResourceLoader::load, DEFVAL(""), DEFVAL(CACHE_MODE_REUSE)); + ClassDB::bind_method(D_METHOD("load_whitelisted", "path", "external_path_whitelist", "type_whitelist", "type_hint", "cache_mode"), &ResourceLoader::load_whitelisted, DEFVAL(""), DEFVAL(CACHE_MODE_REUSE)); ClassDB::bind_method(D_METHOD("get_recognized_extensions_for_type", "type"), &ResourceLoader::get_recognized_extensions_for_type); ClassDB::bind_method(D_METHOD("add_resource_format_loader", "format_loader", "at_front"), &ResourceLoader::add_resource_format_loader, DEFVAL(false)); ClassDB::bind_method(D_METHOD("remove_resource_format_loader", "format_loader"), &ResourceLoader::remove_resource_format_loader); diff --git a/core/core_bind.h b/core/core_bind.h index b96dc56b0dc0..90e8c219f52c 100644 --- a/core/core_bind.h +++ b/core/core_bind.h @@ -70,10 +70,12 @@ class ResourceLoader : public Object { static ResourceLoader *get_singleton() { return singleton; } Error load_threaded_request(const String &p_path, const String &p_type_hint = "", bool p_use_sub_threads = false, CacheMode p_cache_mode = CACHE_MODE_REUSE); + Error load_threaded_request_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint = "", bool p_use_sub_threads = false, CacheMode p_cache_mode = CACHE_MODE_REUSE); ThreadLoadStatus load_threaded_get_status(const String &p_path, Array r_progress = ClassDB::default_array_arg); Ref load_threaded_get(const String &p_path); Ref load(const String &p_path, const String &p_type_hint = "", CacheMode p_cache_mode = CACHE_MODE_REUSE); + Ref load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint = "", CacheMode p_cache_mode = CACHE_MODE_REUSE); Vector get_recognized_extensions_for_type(const String &p_type); void add_resource_format_loader(Ref p_format_loader, bool p_at_front); void remove_resource_format_loader(Ref p_format_loader); diff --git a/core/io/resource_format_binary.cpp b/core/io/resource_format_binary.cpp index 891510e3dc03..e9a4021594e6 100644 --- a/core/io/resource_format_binary.cpp +++ b/core/io/resource_format_binary.cpp @@ -408,6 +408,7 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) { } //always use internal cache for loading internal resources + if (!internal_index_cache.has(path)) { WARN_PRINT(vformat("Couldn't load resource (no cache): %s.", path)); r_v = Variant(); @@ -426,14 +427,24 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) { path = ProjectSettings::get_singleton()->localize_path(res_path.get_base_dir().path_join(path)); } - if (remaps.find(path)) { + if (remaps.has(path)) { path = remaps[path]; } + if (using_whitelist && !external_path_whitelist.has(path)) { + WARN_PRINT(vformat("Blocked path not on whitelist: %s.", path)); + r_v = Variant(); + error = ERR_FILE_CANT_OPEN; + return error; + } + Ref res = ResourceLoader::load(path, exttype, cache_mode_for_external); if (res.is_null()) { - WARN_PRINT(vformat("Couldn't load resource: %s.", path)); + ERR_PRINT(vformat("Couldn't load resource: %s.", path)); + r_v = Variant(); + error = ERR_FILE_CANT_OPEN; + return error; } r_v = res; @@ -443,8 +454,10 @@ Error ResourceLoaderBinary::parse_variant(Variant &r_v) { int erindex = f->get_32(); if (erindex < 0 || erindex >= external_resources.size()) { - WARN_PRINT("Broken external resource! (index out of size)"); + ERR_PRINT("Broken external resource! (index out of size)"); r_v = Variant(); + error = ERR_FILE_CORRUPT; + return error; } else { Ref &load_token = external_resources.write[erindex].load_token; if (load_token.is_valid()) { // If not valid, it's OK since then we know this load accepts broken dependencies. @@ -696,8 +709,9 @@ Error ResourceLoaderBinary::load() { } external_resources.write[i].path = path; //remap happens here, not on load because on load it can actually be used for filesystem dock resource remap - external_resources.write[i].load_token = ResourceLoader::_load_start(path, external_resources[i].type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, cache_mode_for_external); - if (external_resources[i].load_token.is_null()) { + external_resources.write[i].load_token = ResourceLoader::_load_start(path, external_resources[i].type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, cache_mode_for_external, false, false, Dictionary(), Dictionary()); + + if (!external_resources[i].load_token.is_valid()) { if (!ResourceLoader::get_abort_on_missing_resources()) { ResourceLoader::notify_dependency_error(local_path, path, external_resources[i].type); } else { @@ -768,7 +782,10 @@ Error ResourceLoaderBinary::load() { if (res.is_null()) { //did not replace - Object *obj = ClassDB::instantiate(t); + Object *obj = nullptr; + if (!using_whitelist || type_whitelist.has(t)) { + obj = ClassDB::instantiate(t); + } if (!obj) { if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) { //create a missing resource @@ -1246,6 +1263,41 @@ Ref ResourceFormatLoaderBinary::load(const String &p_path, const Strin String path = !p_original_path.is_empty() ? p_original_path : p_path; loader.local_path = ProjectSettings::get_singleton()->localize_path(path); loader.res_path = loader.local_path; + loader.using_whitelist = false; + loader.open(f); + + err = loader.load(); + + if (r_error) { + *r_error = err; + } + + if (err) { + return Ref(); + } + return loader.resource; +} + +Ref ResourceFormatLoaderBinary::load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) { + if (r_error) { + *r_error = ERR_FILE_CANT_OPEN; + } + + Error err; + Ref f = FileAccess::open(p_path, FileAccess::READ, &err); + + ERR_FAIL_COND_V_MSG(err != OK, Ref(), "Cannot open file '" + p_path + "'."); + + ResourceLoaderBinary loader; + loader.cache_mode = p_cache_mode; + loader.use_sub_threads = p_use_sub_threads; + loader.progress = r_progress; + String path = !p_original_path.is_empty() ? p_original_path : p_path; + loader.local_path = ProjectSettings::get_singleton()->localize_path(path); + loader.res_path = loader.local_path; + loader.using_whitelist = true; + loader.external_path_whitelist = p_external_path_whitelist; + loader.type_whitelist = p_type_whitelist; loader.open(f); err = loader.load(); diff --git a/core/io/resource_format_binary.h b/core/io/resource_format_binary.h index ec8d7ead5d2f..9023a8caea2f 100644 --- a/core/io/resource_format_binary.h +++ b/core/io/resource_format_binary.h @@ -84,6 +84,10 @@ class ResourceLoaderBinary { HashMap remaps; Error error = OK; + bool using_whitelist = false; + Dictionary external_path_whitelist; + Dictionary type_whitelist; + ResourceFormatLoader::CacheMode cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE; ResourceFormatLoader::CacheMode cache_mode_for_external = ResourceFormatLoader::CACHE_MODE_REUSE; @@ -111,6 +115,7 @@ class ResourceLoaderBinary { class ResourceFormatLoaderBinary : public ResourceFormatLoader { public: virtual Ref load(const String &p_path, const String &p_original_path = "", Error *r_error = nullptr, bool p_use_sub_threads = false, float *r_progress = nullptr, CacheMode p_cache_mode = CACHE_MODE_REUSE) override; + virtual Ref load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_original_path = "", Error *r_error = nullptr, bool p_use_sub_threads = false, float *r_progress = nullptr, CacheMode p_cache_mode = CACHE_MODE_REUSE) override; virtual void get_recognized_extensions_for_type(const String &p_type, List *p_extensions) const override; virtual void get_recognized_extensions(List *p_extensions) const override; virtual bool handles_type(const String &p_type) const override; diff --git a/core/io/resource_importer.cpp b/core/io/resource_importer.cpp index b7a14f2b8807..54d97d4f186a 100644 --- a/core/io/resource_importer.cpp +++ b/core/io/resource_importer.cpp @@ -171,7 +171,7 @@ Ref ResourceFormatImporter::load_internal(const String &p_path, Error } } - Ref res = ResourceLoader::_load(pat.path, p_path, pat.type, p_cache_mode, r_error, p_use_sub_threads, r_progress); + Ref res = ResourceLoader::_load(pat.path, p_path, pat.type, p_cache_mode, false, Dictionary(), Dictionary(), r_error, p_use_sub_threads, r_progress); #ifdef TOOLS_ENABLED if (res.is_valid()) { diff --git a/core/io/resource_loader.cpp b/core/io/resource_loader.cpp index 7cc91ef4318b..3d1c873ab943 100644 --- a/core/io/resource_loader.cpp +++ b/core/io/resource_loader.cpp @@ -279,7 +279,7 @@ ResourceLoader::LoadToken::~LoadToken() { clear(); } -Ref ResourceLoader::_load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error, bool p_use_sub_threads, float *r_progress) { +Ref ResourceLoader::_load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, bool p_using_whitelist, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, Error *r_error, bool p_use_sub_threads, float *r_progress) { const String &original_path = p_original_path.is_empty() ? p_path : p_original_path; load_nesting++; if (load_paths_stack.size()) { @@ -304,7 +304,11 @@ Ref ResourceLoader::_load(const String &p_path, const String &p_origin continue; } found = true; - res = loader[i]->load(p_path, original_path, r_error, p_use_sub_threads, r_progress, p_cache_mode); + if (p_using_whitelist) { + res = loader[i]->load_whitelisted(p_path, p_external_path_whitelist, p_type_whitelist, p_original_path.is_valid_filename() ? p_path : p_original_path, r_error, p_use_sub_threads, r_progress, p_cache_mode); + } else { + res = loader[i]->load(p_path, p_original_path.is_valid_filename() ? p_path : p_original_path, r_error, p_use_sub_threads, r_progress, p_cache_mode); + } if (res.is_valid()) { break; } @@ -386,7 +390,17 @@ void ResourceLoader::_run_load_task(void *p_userdata) { const String &remapped_path = _path_remap(load_task.local_path, &xl_remapped); Error load_err = OK; - Ref res = _load(remapped_path, remapped_path != load_task.local_path ? load_task.local_path : String(), load_task.type_hint, load_task.cache_mode, &load_err, load_task.use_sub_threads, &load_task.progress); + Ref res = _load(remapped_path, + remapped_path != load_task.local_path ? load_task.local_path : String(), + load_task.type_hint, + load_task.cache_mode, + load_task.using_whitelist, + load_task.external_path_whitelist, + load_task.type_whitelist, + &load_err, + load_task.use_sub_threads, + &load_task.progress); + if (MessageQueue::get_singleton() != MessageQueue::get_main_singleton()) { MessageQueue::get_singleton()->flush(); } @@ -507,7 +521,12 @@ String ResourceLoader::_validate_local_path(const String &p_path) { } Error ResourceLoader::load_threaded_request(const String &p_path, const String &p_type_hint, bool p_use_sub_threads, ResourceFormatLoader::CacheMode p_cache_mode) { - Ref token = _load_start(p_path, p_type_hint, p_use_sub_threads ? LOAD_THREAD_DISTRIBUTE : LOAD_THREAD_SPAWN_SINGLE, p_cache_mode, true); + Ref token = _load_start(p_path, p_type_hint, p_use_sub_threads ? LOAD_THREAD_DISTRIBUTE : LOAD_THREAD_SPAWN_SINGLE, p_cache_mode, true, false, Dictionary(), Dictionary()); + return token.is_valid() ? OK : FAILED; +} + +Error ResourceLoader::load_threaded_request_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint, bool p_use_sub_threads, ResourceFormatLoader::CacheMode p_cache_mode) { + Ref token = _load_start(p_path, p_type_hint, p_use_sub_threads ? LOAD_THREAD_DISTRIBUTE : LOAD_THREAD_SPAWN_SINGLE, p_cache_mode, true, true, p_external_path_whitelist, p_type_whitelist); return token.is_valid() ? OK : FAILED; } @@ -544,8 +563,33 @@ Ref ResourceLoader::load(const String &p_path, const String &p_type_hi // cyclic load detection and awaiting. thread_mode = LOAD_THREAD_SPAWN_SINGLE; } - Ref load_token = _load_start(p_path, p_type_hint, thread_mode, p_cache_mode); - if (load_token.is_null()) { + Ref load_token = _load_start(p_path, p_type_hint, thread_mode, p_cache_mode, false, false, Dictionary(), Dictionary()); + if (!load_token.is_valid()) { + if (r_error) { + *r_error = FAILED; + } + return Ref(); + } + + Ref res = _load_complete(*load_token.ptr(), r_error); + return res; +} + +Ref ResourceLoader::load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error) { + if (r_error) { + *r_error = OK; + } + + LoadThreadMode thread_mode = LOAD_THREAD_FROM_CURRENT; + if (WorkerThreadPool::get_singleton()->get_caller_task_id() != WorkerThreadPool::INVALID_TASK_ID) { + // If user is initiating a single-threaded load from a WorkerThreadPool task, + // we instead spawn a new task so there's a precondition that a load in a pool task + // is always initiated by the engine. That makes certain aspects simpler, such as + // cyclic load detection and awaiting. + thread_mode = LOAD_THREAD_SPAWN_SINGLE; + } + Ref load_token = _load_start(p_path, p_type_hint, thread_mode, p_cache_mode, false, true, p_external_path_whitelist, p_type_whitelist); + if (!load_token.is_valid()) { if (r_error) { *r_error = FAILED; } @@ -556,9 +600,14 @@ Ref ResourceLoader::load(const String &p_path, const String &p_type_hi return res; } -Ref ResourceLoader::_load_start(const String &p_path, const String &p_type_hint, LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode, bool p_for_user) { +Ref ResourceLoader::_load_start(const String &p_path, const String &p_type_hint, LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode, bool p_for_user, bool p_use_whitelist, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist) { String local_path = _validate_local_path(p_path); + if (p_use_whitelist && !p_external_path_whitelist.has(local_path)) { + WARN_PRINT(vformat("Blocked path not on whitelist: %s.", local_path)); + return Ref(); + } + bool ignoring_cache = p_cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE || p_cache_mode == ResourceFormatLoader::CACHE_MODE_IGNORE_DEEP; Ref load_token; @@ -605,6 +654,9 @@ Ref ResourceLoader::_load_start(const String &p_path, load_task.type_hint = p_type_hint; load_task.cache_mode = p_cache_mode; load_task.use_sub_threads = p_thread_mode == LOAD_THREAD_DISTRIBUTE; + load_task.using_whitelist = p_use_whitelist; + load_task.external_path_whitelist = p_external_path_whitelist; + load_task.type_whitelist = p_type_whitelist; if (p_cache_mode == ResourceFormatLoader::CACHE_MODE_REUSE) { Ref existing = ResourceCache::get_ref(local_path); if (existing.is_valid()) { @@ -1573,3 +1625,7 @@ HashMap> ResourceLoader::translation_remaps; HashMap ResourceLoader::path_remaps; ResourceLoaderImport ResourceLoader::import = nullptr; + +Ref ResourceFormatLoader::load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) { + return Ref(); +} diff --git a/core/io/resource_loader.h b/core/io/resource_loader.h index 56052b6a6fc4..48a7ca5ec7cb 100644 --- a/core/io/resource_loader.h +++ b/core/io/resource_loader.h @@ -75,6 +75,7 @@ class ResourceFormatLoader : public RefCounted { public: virtual Ref load(const String &p_path, const String &p_original_path = "", Error *r_error = nullptr, bool p_use_sub_threads = false, float *r_progress = nullptr, CacheMode p_cache_mode = CACHE_MODE_REUSE); + virtual Ref load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_original_path = "", Error *r_error = nullptr, bool p_use_sub_threads = false, float *r_progress = nullptr, CacheMode p_cache_mode = CACHE_MODE_REUSE); virtual bool exists(const String &p_path) const; virtual void get_recognized_extensions(List *p_extensions) const; virtual void get_recognized_extensions_for_type(const String &p_type, List *p_extensions) const; @@ -140,7 +141,7 @@ class ResourceLoader { static const int BINARY_MUTEX_TAG = 1; - static Ref _load_start(const String &p_path, const String &p_type_hint, LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode, bool p_for_user = false); + static Ref _load_start(const String &p_path, const String &p_type_hint, LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode, bool p_for_user, bool p_use_whitelist, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist); static Ref _load_complete(LoadToken &p_load_token, Error *r_error); private: @@ -169,7 +170,7 @@ class ResourceLoader { friend class ResourceFormatImporter; - static Ref _load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error *r_error, bool p_use_sub_threads, float *r_progress); + static Ref _load(const String &p_path, const String &p_original_path, const String &p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, bool p_using_whitelist, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, Error *r_error, bool p_use_sub_threads, float *r_progress); static ResourceLoadedCallback _loaded_callback; @@ -193,6 +194,7 @@ class ResourceLoader { Error error = OK; Ref resource; bool use_sub_threads = false; + bool using_whitelist = false; HashSet sub_tasks; struct ResourceChangedConnection { @@ -201,6 +203,8 @@ class ResourceLoader { uint32_t flags = 0; }; LocalVector resource_changed_connections; + Dictionary external_path_whitelist; + Dictionary type_whitelist; }; static void _run_load_task(void *p_userdata); @@ -220,11 +224,14 @@ class ResourceLoader { static float _dependency_get_progress(const String &p_path); + static Error _load_threaded_request_whitelisted_int(const String &p_path, const String &p_type_hint, bool p_use_sub_threads, ResourceFormatLoader::CacheMode p_cache_mode, bool p_use_whitelist, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist); + static bool _ensure_load_progress(); static String _validate_local_path(const String &p_path); public: + static Error load_threaded_request_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint = "", bool p_use_sub_threads = false, ResourceFormatLoader::CacheMode p_cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE); static Error load_threaded_request(const String &p_path, const String &p_type_hint = "", bool p_use_sub_threads = false, ResourceFormatLoader::CacheMode p_cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE); static ThreadLoadStatus load_threaded_get_status(const String &p_path, float *r_progress = nullptr); static Ref load_threaded_get(const String &p_path, Error *r_error = nullptr); @@ -235,6 +242,7 @@ class ResourceLoader { static void resource_changed_disconnect(Resource *p_source, const Callable &p_callable); static void resource_changed_emit(Resource *p_source); + static Ref load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_type_hint = "", ResourceFormatLoader::CacheMode p_cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE, Error *r_error = nullptr); static Ref load(const String &p_path, const String &p_type_hint = "", ResourceFormatLoader::CacheMode p_cache_mode = ResourceFormatLoader::CACHE_MODE_REUSE, Error *r_error = nullptr); static bool exists(const String &p_path, const String &p_type_hint = ""); diff --git a/doc/classes/ResourceLoader.xml b/doc/classes/ResourceLoader.xml index f8096dc7b141..3d518b126bf1 100644 --- a/doc/classes/ResourceLoader.xml +++ b/doc/classes/ResourceLoader.xml @@ -127,6 +127,33 @@ The [param cache_mode] property defines whether and how the cache should be used or updated when loading the resource. See [enum CacheMode] for details. + + + + + + + + + + Loads the resource using threads, but only if the resource's path is in the provided whitelist. The whitelist is a dictionary where the keys are the paths to be whitelisted. + The [param type_whitelist] is a dictionary where the keys are the types of resources to be whitelisted. + If [param use_sub_threads] is [code]true[/code], multiple threads will be used to load the resource, which makes loading faster, but may affect the main thread (and thus cause game slowdowns). + The [param cache_mode] property defines whether and how the cache should be used or updated when loading the resource. See [enum CacheMode] for details. + + + + + + + + + + + Loads a resource at the given [param path], caching the result for further access, but only if the resource's path is in the provided whitelist. The whitelist is a dictionary where the keys are the paths to be whitelisted. + The [param type_whitelist] is a dictionary where the keys are the types of resources to be whitelisted. + + diff --git a/scene/resources/resource_format_text.cpp b/scene/resources/resource_format_text.cpp index f1cfb5c5f515..015d608f5821 100644 --- a/scene/resources/resource_format_text.cpp +++ b/scene/resources/resource_format_text.cpp @@ -140,6 +140,7 @@ Error ResourceLoaderText::_parse_ext_resource(VariantParser::Stream *p_stream, R String path = ext_resources[id].path; String type = ext_resources[id].type; + Ref &load_token = ext_resources[id].load_token; if (load_token.is_valid()) { // If not valid, it's OK since then we know this load accepts broken dependencies. @@ -398,7 +399,6 @@ Ref ResourceLoaderText::_parse_node_tag(VariantParser::ResourcePars } } } - Error ResourceLoaderText::load() { if (error != OK) { return error; @@ -461,9 +461,24 @@ Error ResourceLoaderText::load() { path = remaps[path]; } - ext_resources[id].path = path; - ext_resources[id].type = type; - ext_resources[id].load_token = ResourceLoader::_load_start(path, type, use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, cache_mode_for_external); + // Added whitelist check. + if (!using_whitelist || external_path_whitelist.has(path)) { + ext_resources[id].load_token = ResourceLoader::_load_start( + path, + type, + use_sub_threads ? ResourceLoader::LOAD_THREAD_DISTRIBUTE : ResourceLoader::LOAD_THREAD_FROM_CURRENT, + cache_mode_for_external, + false, + using_whitelist, + external_path_whitelist, + type_whitelist); + } else { + error = ERR_FILE_CANT_OPEN; + error_text = "External resource path is not in the whitelist: " + path; + _printerr(); + return error; + } + if (ext_resources[id].load_token.is_null()) { if (ResourceLoader::get_abort_on_missing_resources()) { error = ERR_FILE_CORRUPT; @@ -513,8 +528,6 @@ Error ResourceLoaderText::load() { String path = local_path + "::" + id; - //bool exists=ResourceCache::has(path); - Ref res; bool do_assign = false; @@ -530,40 +543,50 @@ Error ResourceLoaderText::load() { MissingResource *missing_resource = nullptr; - if (res.is_null()) { //not reuse - Ref cache = ResourceCache::get_ref(path); - if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE && cache.is_valid()) { //only if it doesn't exist - //cached, do not assign - res = cache; - } else { - //create + if (!using_whitelist || external_path_whitelist.has(path)) { + if (res.is_null()) { //not reuse + Ref cache = ResourceCache::get_ref(path); + if (cache_mode != ResourceFormatLoader::CACHE_MODE_IGNORE && cache.is_valid()) { //only if it doesn't exist + //cached, do not assign + res = cache; + } else { + //create - Object *obj = ClassDB::instantiate(type); - if (!obj) { - if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) { - missing_resource = memnew(MissingResource); - missing_resource->set_original_class(type); - missing_resource->set_recording_properties(true); - obj = missing_resource; - } else { - error_text = vformat("Can't create sub resource of type '%s'", type); + Object *obj = nullptr; + if (!using_whitelist || type_whitelist.has(type)) { + obj = ClassDB::instantiate(type); + } + if (!obj) { + if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) { + missing_resource = memnew(MissingResource); + missing_resource->set_original_class(type); + missing_resource->set_recording_properties(true); + obj = missing_resource; + } else { + error_text = vformat("Can't create sub resource of type '%s'", type); + _printerr(); + error = ERR_FILE_CORRUPT; + return error; + } + } + + Resource *r = Object::cast_to(obj); + if (!r) { + error_text = vformat("Can't create sub resource of type '%s' as it's not a resource type", type); _printerr(); error = ERR_FILE_CORRUPT; return error; } - } - Resource *r = Object::cast_to(obj); - if (!r) { - error_text = vformat("Can't create sub resource of type '%s' as it's not a resource type", type); - _printerr(); - error = ERR_FILE_CORRUPT; - return error; + res = Ref(r); + do_assign = true; } - - res = Ref(r); - do_assign = true; } + } else { + error = ERR_FILE_CANT_OPEN; + error_text = "Sub resource path is not in the whitelist: " + path; + _printerr(); + return error; } resource_current++; @@ -684,7 +707,10 @@ Error ResourceLoaderText::load() { } if (resource.is_null()) { - Object *obj = ClassDB::instantiate(res_type); + Object *obj = nullptr; + if (!using_whitelist || type_whitelist.has(res_type)) { + obj = ClassDB::instantiate(res_type); + } if (!obj) { if (ResourceLoader::is_creating_missing_resources_if_class_unavailable_enabled()) { missing_resource = memnew(MissingResource); @@ -1409,6 +1435,7 @@ Ref ResourceFormatLoaderText::load(const String &p_path, const String loader.local_path = ProjectSettings::get_singleton()->localize_path(path); loader.progress = r_progress; loader.res_path = loader.local_path; + loader.using_whitelist = false; loader.open(f); err = loader.load(); if (r_error) { @@ -1421,6 +1448,40 @@ Ref ResourceFormatLoaderText::load(const String &p_path, const String } } +Ref ResourceFormatLoaderText::load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_original_path, Error *r_error, bool p_use_sub_threads, float *r_progress, CacheMode p_cache_mode) { + if (r_error) { + *r_error = ERR_FILE_CANT_OPEN; + } + + Error err; + Ref f = FileAccess::open(p_path, FileAccess::READ, &err); + + ERR_FAIL_COND_V_MSG(err != OK, Ref(), "Cannot open file '" + p_path + "'."); + + ResourceLoaderText loader; + loader.cache_mode = p_cache_mode; + loader.use_sub_threads = p_use_sub_threads; + loader.progress = r_progress; + String path = !p_original_path.is_empty() ? p_original_path : p_path; + loader.local_path = ProjectSettings::get_singleton()->localize_path(path); + loader.res_path = loader.local_path; + loader.using_whitelist = true; + loader.external_path_whitelist = p_external_path_whitelist; + loader.type_whitelist = p_type_whitelist; + loader.open(f); + + err = loader.load(); + + if (r_error) { + *r_error = err; + } + + if (err) { + return Ref(); + } + return loader.resource; +} + void ResourceFormatLoaderText::get_recognized_extensions_for_type(const String &p_type, List *p_extensions) const { if (p_type.is_empty()) { get_recognized_extensions(p_extensions); diff --git a/scene/resources/resource_format_text.h b/scene/resources/resource_format_text.h index 4c0bf3d91703..39a8bc1f4e2e 100644 --- a/scene/resources/resource_format_text.h +++ b/scene/resources/resource_format_text.h @@ -120,6 +120,10 @@ class ResourceLoaderText { Error error = OK; + bool using_whitelist = false; + Dictionary external_path_whitelist; + Dictionary type_whitelist; + Ref resource; Ref _parse_node_tag(VariantParser::ResourceParser &parser); @@ -147,6 +151,7 @@ class ResourceFormatLoaderText : public ResourceFormatLoader { public: static ResourceFormatLoaderText *singleton; virtual Ref load(const String &p_path, const String &p_original_path = "", Error *r_error = nullptr, bool p_use_sub_threads = false, float *r_progress = nullptr, CacheMode p_cache_mode = CACHE_MODE_REUSE) override; + virtual Ref load_whitelisted(const String &p_path, Dictionary p_external_path_whitelist, Dictionary p_type_whitelist, const String &p_original_path = "", Error *r_error = nullptr, bool p_use_sub_threads = false, float *r_progress = nullptr, CacheMode p_cache_mode = CACHE_MODE_REUSE) override; virtual void get_recognized_extensions_for_type(const String &p_type, List *p_extensions) const override; virtual void get_recognized_extensions(List *p_extensions) const override; virtual bool handles_type(const String &p_type) const override; diff --git a/tests/data/load_resource_golden_path/authorized_resource.res b/tests/data/load_resource_golden_path/authorized_resource.res new file mode 100644 index 0000000000000000000000000000000000000000..7b86f6e314d6312d1a0f7570aabe05619c506909 GIT binary patch literal 397 zcmV;80doFQQ$s@n000005C8z50ssJY0RR9fwJ-f(p8;J907el!Ghl7f0WdJ5RPI3; zF#!MoVL2WCAhKjcl9w&9MD80KX7e<#WWA8-4q_M!@&CjO-DJ_GD*!0~F90#O6E?Rn z)p52_`Tym=!Xc0ojjWhJ{|9a%p`0~`X7r_RYP|Ws@LwR{5C03^*kc!CYow_CPjGb& zi^&VuHC(5)^4ix8F2>r|`_FK-+Lds<)CHZK+QKN(#0(|m5g3d$VPzC)0Jq|Z-rAu{4n z+5DMD<%UqtQuL literal 0 HcmV?d00001 diff --git a/tests/data/load_resource_golden_path/authorized_resource.tres b/tests/data/load_resource_golden_path/authorized_resource.tres new file mode 100644 index 000000000000..0c33102749ab --- /dev/null +++ b/tests/data/load_resource_golden_path/authorized_resource.tres @@ -0,0 +1,11 @@ +[gd_resource type="GradientTexture2D" load_steps=2 format=3 uid="uid://cgyatudp08mhw"] + +[sub_resource type="Gradient" id="Gradient_m6m551"] +interpolation_mode = 2 +offsets = PackedFloat32Array(0, 0.2, 1) +colors = PackedColorArray(0, 0, 0, 1, 0, 1, 0, 1, 1, 0, 0, 1) + +[resource] +gradient = SubResource("Gradient_m6m551") +fill_from = Vector2(0.01, 0.5) +fill_to = Vector2(0.5, 0.5) diff --git a/tests/data/load_resource_malicious_path/trojan_resource.res b/tests/data/load_resource_malicious_path/trojan_resource.res new file mode 100644 index 0000000000000000000000000000000000000000..4301471d9720b1d02e6247008570da0134b1417a GIT binary patch literal 398 zcmV;90df9PQ$s@n000005C8zG0ssJZ0RR9fwJ-f(ssUvS05;J(GeG4g9jV;zR=6FIQ{lsfq(?RdD*!0~E&!8lv)J55 zrjDD7GyVP>ztHCvB5=0Z0dXvFIyF6y2;Jh}m=!OL+ z(%umXSRxh#4*|z)Y`Y$XC~s4Q9dwpvs;Z(h zOT=c2zEsQM%E&wY4E&vSQYNe^% z8k@vXSCSXcmH!*gB1kfzNQvct!8t5eUJbGzT`hD?M*kiD6Ab(T`(8W9+5A6vTTn`K z_ReYI{{~;FSyLrPmL$H8DXkcGi)_j53jTliQmqPZd&0O4A4gr|$QWj3D4$o2qq!S=Afe85(5-b$R19>R^S0K-? z(eXK%5QzTF=gO|w4wU@*YV38hv|1N=qX9P{V1gmX$UN}`PGBTpDUWt(u;|IYpdCN7 zq5wa=Zgh188fh<#0v;#~U@BmMVK7*VXDsp?uBgOsy+O?azRNYi^}*z2zm3&t+}nYi zpc(%H+j%3@N?%49z?`cw84flGm7nWcvCZiQ{s+2fTZ-B7W8YdAXTHoE&k3wx0a8;# ELo^4l>i_@% literal 0 HcmV?d00001 diff --git a/tests/data/load_resource_whitelisted_golden_path/authorized_resource.tres b/tests/data/load_resource_whitelisted_golden_path/authorized_resource.tres new file mode 100644 index 000000000000..d678b1965e41 --- /dev/null +++ b/tests/data/load_resource_whitelisted_golden_path/authorized_resource.tres @@ -0,0 +1,12 @@ +[gd_resource type="GradientTexture2D" load_steps=2 format=3 uid="uid://cgyatudp08mhy"] + +[sub_resource type="Gradient" id="Gradient_m6m553"] +interpolation_mode = 2 +offsets = PackedFloat32Array(0, 0.2, 1) +colors = PackedColorArray(0, 0, 0, 1, 1, 0, 0, 1, 0, 1, 0, 1) + +[resource] +gradient = SubResource("Gradient_m6m553") +fill = 1 +fill_from = Vector2(0.01, 0.5) +fill_to = Vector2(0.5, 0.5) diff --git a/tests/data/load_resource_whitelisted_malicious_path/trojan_resource.res b/tests/data/load_resource_whitelisted_malicious_path/trojan_resource.res new file mode 100644 index 0000000000000000000000000000000000000000..93670d3aec35ab3025446f8bbeef23421409e798 GIT binary patch literal 411 zcmV;M0c8GCQ$s@n000005C8zS0ssJm0RR9fwJ-f(wgJrx00vS%G+=Gg0l*Xiy9AnH z7---SB-UgRtrA%>BFS!CVu{>0Hq7QJ#kLVlf{tVU>c6i9c&i!IQY`>005AZTx|rJu zn_HObINPZF|Kb0_J4J;0_@IRRzg$D1oOOq8^rdfVy!p@Ye?Z_5RZq{;`CstH9=jM@ zBSr0hf~#v-OkTJS;ySIB*S>CWG1k7`|Awp8E`@88c6?C#Ri2#M!YIl_O@-tY7>s3M zWfWxqnqtb^X)RM}a0nmu-*TR+)R=YEd0GV+91Tt=K?;F*oNPuMK0m_2r6PeOki>tB z1o|&BN=zaGHoxXkxhJm1vD{JB4a=DAa}PEv12!OFf-$J5Jn;n9avWimN6R!&6;0(%u&Z?n6MpSiqPeVXzX9Sl~CdV)?B%rdhyuw=bApFmW$$gLV3Q zIgrz6#=pRRz7reWeWNm9u4#-52L(dh&)uxpTp9?k2D%Vh`t0)L9?E>tH da = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); + da->make_dir_recursive(project_folder.path_join(".godot").path_join("imported")); + // Initialize res:// to `project_folder`. + TestProjectSettingsInternalsAccessor::resource_path() = project_folder; + err = ProjectSettings::get_singleton()->setup(project_folder, String(), true); + + if (p_copy_target.is_empty()) { + return; + } + + // Copy all the necessary test data files to the res:// directory. + da = DirAccess::create(DirAccess::ACCESS_FILESYSTEM); + String test_data = String("tests/data").path_join(p_test); + da = DirAccess::open(test_data); + CHECK_MESSAGE(da.is_valid(), "Unable to open folder."); + da->list_dir_begin(); + for (String item = da->get_next(); !item.is_empty(); item = da->get_next()) { + if (!FileAccess::exists(test_data.path_join(item))) { + continue; + } + Ref output = FileAccess::open(p_copy_target.path_join(item), FileAccess::WRITE, &err); + CHECK_MESSAGE(err == OK, "Unable to open output file."); + output->store_buffer(FileAccess::get_file_as_bytes(test_data.path_join(item))); + output->close(); + } + da->list_dir_end(); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Resource - Load invalid path") { + init("null_path"); + Ref resource = ResourceLoader::load(""); + CHECK_FALSE(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Text Resource - Load valid path") { + init("load_resource_golden_path", "res://"); + Ref resource = ResourceLoader::load("authorized_resource.tres", "Texture2D"); + CHECK(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Text Resource - Malicious Path") { + init("load_resource_malicious_path", "res://"); + Ref resource = ResourceLoader::load("trojan_resource.tres", "Texture2D"); + CHECK(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Binary Resource - Load valid path") { + init("load_resource_golden_path", "res://"); + Ref resource = ResourceLoader::load("authorized_resource.res", "Texture2D"); + CHECK(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Binary Resource - Malicious Path") { + init("load_resource_malicious_path", "res://"); + Ref resource = ResourceLoader::load("trojan_resource.res", "Texture2D"); + CHECK(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Text Resource Whitelisted - Load invalid path") { + init("null_path"); + Dictionary ext_whitelist; + Dictionary type_whitelist; + Ref resource = ResourceLoader::load_whitelisted("", ext_whitelist, type_whitelist); + CHECK_FALSE(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Text Resource Whitelisted - Load valid path") { + init("load_resource_whitelisted_golden_path", "res://"); + Dictionary ext_whitelist; + ext_whitelist["res://authorized_resource.tres"] = true; + ext_whitelist["res://authorized_resource.tres::Gradient_m6m553"] = true; + Dictionary type_whitelist; + type_whitelist["GradientTexture2D"] = true; + type_whitelist["Gradient"] = true; + Ref resource = ResourceLoader::load_whitelisted( + "res://authorized_resource.tres", ext_whitelist, type_whitelist, "Texture2D"); + CHECK(resource.is_valid()); + REQUIRE_EQ(resource->get_class_static(), "Texture2D"); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Text Resource Whitelisted - No allowed paths in the whitelist") { + init("load_resource_whitelisted_malicious_path", "res://"); + Dictionary ext_whitelist; + Dictionary type_whitelist; + type_whitelist["GradientTexture2D"] = true; + type_whitelist["Gradient"] = true; + Ref resource = ResourceLoader::load_whitelisted( + "res://trojan_resource.tres", ext_whitelist, type_whitelist, "Texture2D"); + CHECK_FALSE(resource.is_valid()); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Binary Resource Whitelisted - Load valid path") { + init("load_resource_whitelisted_golden_path", "res://"); + Dictionary ext_whitelist; + ext_whitelist["res://authorized_resource.res"] = true; + Dictionary type_whitelist; + type_whitelist["GradientTexture2D"] = true; + type_whitelist["Gradient"] = true; + Ref resource = ResourceLoader::load_whitelisted( + "res://authorized_resource.res", ext_whitelist, type_whitelist, "Texture2D"); + CHECK(resource.is_valid()); + REQUIRE_EQ(resource->get_class_static(), "Texture2D"); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Binary Resource Whitelisted - No allowed paths in the whitelist") { + init("load_resource_whitelisted_malicious_path", "res://"); + Dictionary ext_whitelist; + Dictionary type_whitelist; + type_whitelist["GradientTexture2D"] = true; + type_whitelist["Gradient"] = true; + Ref resource = ResourceLoader::load_whitelisted( + "res://trojan_resource.res", ext_whitelist, type_whitelist, "Texture2D"); + CHECK_FALSE_MESSAGE(resource.is_valid(), "The resource is valid. You can now remotely open notepad.exe through remote gdscript execution."); +} + +TEST_CASE("[SceneTree][ResourceLoader] Load Binary Resource Whitelisted - Load valid path but invalid type") { + init("load_resource_whitelisted_golden_path", "res://"); + Dictionary ext_whitelist; + ext_whitelist["res://authorized_resource.res"] = true; + Dictionary type_whitelist; + Ref resource = ResourceLoader::load_whitelisted( + "res://authorized_resource.res", ext_whitelist, type_whitelist, "Texture2D"); + CHECK_FALSE(resource.is_valid()); +} + +} // namespace TestResourceLoader + +#endif // TEST_RESOURCE_LOADER_H diff --git a/tests/test_main.cpp b/tests/test_main.cpp index af334dbbf4a6..2e5d42fb27f9 100644 --- a/tests/test_main.cpp +++ b/tests/test_main.cpp @@ -176,6 +176,7 @@ #include "tests/scene/test_path_3d.h" #include "tests/scene/test_path_follow_3d.h" #include "tests/scene/test_primitives.h" +#include "tests/scene/test_resource_loader.h" #include "tests/scene/test_skeleton_3d.h" #include "tests/scene/test_sky.h" #endif // _3D_DISABLED