diff --git a/src/filesystem.cpp b/src/filesystem.cpp index 172c8dda06e3..7f0c02da9103 100644 --- a/src/filesystem.cpp +++ b/src/filesystem.cpp @@ -1206,6 +1206,57 @@ void clear_binary_paths_cache() binary_paths_cache.clear(); } +static std::string resolve_filename(std::string filename, const std::string relative_to) +{ + // Collapses any . or .. components in the filename + if(filename.empty()) { + return filename; + } + // We'll assume it's relative if the first character is a dot. + if(filename[0] == '.') { + filename = relative_to + '/' + filename; + } + // Drop initial slash ("root" directory is an empty string in Wesnoth) + if(filename[0] == '/') { + filename.erase(0, 1); + } + // Resolve /./ + while(true) { + size_t pos = filename.find("/./"); + if(pos == std::string::npos) { + break; + } + filename.erase(pos, 2); + } + // Resolve // + while(true) { + size_t pos = filename.find("//"); + if(pos == std::string::npos) { + break; + } + filename.erase(pos, 1); + } + // Resolve /../ + while(true) { + size_t pos = filename.find("/.."); + if(pos == std::string::npos) { + break; + } + size_t pos2 = filename.find_last_of('/', pos - 1); + if(pos2 == std::string::npos) { + // Means there is no earlier slash + pos2 = filename[0] == '~' ? 1 : 0; + pos += 1; + } + filename.erase(pos, 3); + filename.erase(pos2, pos - pos2); + } + if(filename.find("~../") == 0) { + filename.erase(0, 4); + } + return filename; +} + static bool is_legal_file(const std::string& filename_str) { DBG_FS << "Looking for '" << filename_str << "'.\n"; @@ -1215,8 +1266,8 @@ static bool is_legal_file(const std::string& filename_str) return false; } - if(filename_str.find("..") != std::string::npos) { - ERR_FS << "Illegal path '" << filename_str << "' (\"..\" not allowed).\n"; + if (filename.size() >= 2 && filename[0] == '.' && filename[1] == '.') { + ERR_FS << "Illegal path '" << filename << "' (initial \"..\" not allowed).\n"; return false; } @@ -1282,20 +1333,9 @@ const std::vector& get_binary_paths(const std::string& type) return res; } -std::string get_binary_file_location(const std::string& type, const std::string& filename) +std::string get_binary_file_location(const std::string& type, const std::string& filename_unresolved) { - // We define ".." as "remove everything before" this is needed because - // on the one hand allowing ".." would be a security risk but - // especially for terrains the c++ engine puts a hardcoded "terrain/" before filename - // and there would be no way to "escape" from "terrain/" otherwise. This is not the - // best solution but we cannot remove it without another solution (subtypes maybe?). - - { - std::string::size_type pos = filename.rfind("../"); - if(pos != std::string::npos) { - return get_binary_file_location(type, filename.substr(pos + 3)); - } - } + const std::string& filename = resolve_filename(filename_unresolved, ""); if(!is_legal_file(filename)) { return std::string(); @@ -1317,9 +1357,11 @@ std::string get_binary_file_location(const std::string& type, const std::string& return std::string(); } -std::string get_binary_dir_location(const std::string& type, const std::string& filename) +std::string get_binary_dir_location(const std::string &type, const std::string &filename_unresolved) { - if(!is_legal_file(filename)) { + const std::string& filename = resolve_filename(filename_unresolved, ""); + + if (!is_legal_file(filename)) return std::string(); } @@ -1337,15 +1379,22 @@ std::string get_binary_dir_location(const std::string& type, const std::string& return std::string(); } -std::string get_wml_location(const std::string& filename, const std::string& current_dir) +std::string get_wml_location(const std::string &filename_unresolved, const std::string ¤t_dir) { - if(!is_legal_file(filename)) { + // Special cases first: + if(filename_unresolved == "/") { + return game_config::path; + } else if(filename_unresolved == ".") { + return current_dir.empty() ? game_config::path : current_dir; + } + const std::string& filename = resolve_filename(filename_unresolved, get_short_wml_path(current_dir)); + + if (!is_legal_file(filename)) return std::string(); } assert(game_config::path.empty() == false); - bfs::path fpath(filename); bfs::path result; if(filename[0] == '~') { diff --git a/src/game_config_manager.cpp b/src/game_config_manager.cpp index 143899ec6718..d22b1e2cf607 100644 --- a/src/game_config_manager.cpp +++ b/src/game_config_manager.cpp @@ -214,9 +214,9 @@ void game_config_manager::load_game_config(FORCE_RELOAD_CONFIG force_reload, } const std::string& path = core["path"]; - if (!filesystem::file_exists(filesystem::get_wml_location(path))) { - events::call_in_main_thread([&]() { - gui2::dialogs::wml_error::display( + // get_wml_location() returns empty if the path does not exist + if(filesystem::get_wml_location(path).empty()) { + gui2::dialogs::wml_error::display( _("Error validating data core."), _("Core ID: ") + id + '\n' + _("Core Path: ") + path diff --git a/src/scripting/lua_fileops.cpp b/src/scripting/lua_fileops.cpp index 2fc899d6b9d9..f9937c8cae91 100644 --- a/src/scripting/lua_fileops.cpp +++ b/src/scripting/lua_fileops.cpp @@ -64,54 +64,13 @@ static std::string get_calling_file(lua_State* L) /// @returns true if the filename was successfully resolved. static bool resolve_filename(std::string& filename, std::string currentdir, std::string* rel = nullptr) { - if(filename.size() < 2) { - return false; - } - if(filename[0] == '.' && filename[1] == '/') { - filename = currentdir + filename.substr(1); - } - if(std::find(filename.begin(), filename.end(), '\\') != filename.end()) { - return false; - } - //resolve /./ - while(true) { - std::size_t pos = filename.find("/./"); - if(pos == std::string::npos) { - break; - } - filename = filename.replace(pos, 2, ""); - } - //resolve // - while(true) { - std::size_t pos = filename.find("//"); - if(pos == std::string::npos) { - break; - } - filename = filename.replace(pos, 1, ""); - } - //resolve /../ - while(true) { - std::size_t pos = filename.find("/.."); - if(pos == std::string::npos) { - break; - } - std::size_t pos2 = filename.find_last_of('/', pos - 1); - if(pos2 == std::string::npos || pos2 >= pos) { - return false; - } - filename = filename.replace(pos2, pos- pos2 + 3, ""); - } - if(filename.find("..") != std::string::npos) { - return false; - } - std::string p = filesystem::get_wml_location(filename); - if(p.empty()) { + filename = filesystem::get_wml_location(filename, currentdir); + if(filename.empty()) { return false; } if(rel) { - *rel = filename; + *rel = filesystem::get_short_wml_path(filename); } - filename = p; return true; } diff --git a/src/tests/test_filesystem.cpp b/src/tests/test_filesystem.cpp index 38da37cd2341..545d3536bc4c 100644 --- a/src/tests/test_filesystem.cpp +++ b/src/tests/test_filesystem.cpp @@ -143,11 +143,14 @@ BOOST_AUTO_TEST_CASE( test_fs_binary_path ) BOOST_CHECK_EQUAL( get_binary_dir_location("images", "."), gamedata + "/images/." ); BOOST_CHECK_EQUAL( get_binary_file_location("images", "././././././"), - gamedata + "/images/././././././" ); + gamedata + "/images/" ); BOOST_CHECK_EQUAL( get_binary_file_location("images", "wesnoth-icon.png"), gamedata + "/data/core/images/wesnoth-icon.png" ); + BOOST_CHECK_EQUAL(get_binary_file_location("images", "terrain/../scenery/dwarven-doors-closed.png"), + gamedata + "/data/core/images/scenery/dwarven-doors-closed.png"); + BOOST_CHECK_EQUAL( get_binary_file_location("music", "silence.ogg"), gamedata + "/data/core/music/silence.ogg" ); @@ -157,14 +160,18 @@ BOOST_AUTO_TEST_CASE( test_fs_binary_path ) BOOST_CHECK_EQUAL( get_independent_image_path("wesnoth-icon.png"), "data/core/images/wesnoth-icon.png" ); - // Inexistent paths are resolved empty. + // Nonexistent paths are resolved empty. BOOST_CHECK( get_binary_dir_location("images", "").empty() ); - BOOST_CHECK( get_binary_dir_location("inexistent_resource_type", "").empty() ); + BOOST_CHECK( get_binary_dir_location("nonexistent_resource_type", "").empty() ); BOOST_CHECK( get_binary_file_location("image", "wesnoth-icon.png").empty() ); BOOST_CHECK( get_binary_file_location("images", "bunnies_and_ponies_and_rainbows_oh_em_gee.psd").empty() ); BOOST_CHECK( get_binary_file_location("music", "this_track_does_not_exist.aiff").empty() ); BOOST_CHECK( get_binary_file_location("sounds", "rude_noises.aiff").empty() ); BOOST_CHECK( get_independent_image_path("dopefish.txt").empty() ); + + // Unsafe paths are resolved empty. + BOOST_CHECK(get_binary_file_location("images", "..").empty()); + BOOST_CHECK(get_binary_file_location("images", "../sounds/bell.wav").empty()) } BOOST_AUTO_TEST_CASE( test_fs_wml_path ) @@ -175,12 +182,24 @@ BOOST_AUTO_TEST_CASE( test_fs_wml_path ) BOOST_CHECK_EQUAL( get_wml_location("_main.cfg"), gamedata + "/data/_main.cfg" ); BOOST_CHECK_EQUAL( get_wml_location("core/_main.cfg"), gamedata + "/data/core/_main.cfg" ); - BOOST_CHECK_EQUAL( get_wml_location("."), gamedata + "/data/." ); + BOOST_CHECK_EQUAL(get_wml_location("core/units"), gamedata + "/data/core/units"); + BOOST_CHECK_EQUAL( get_wml_location("."), gamedata + "/data" ); + BOOST_CHECK_EQUAL(get_wml_location("/"), gamedata + "/data") BOOST_CHECK_EQUAL( get_wml_location("~/"), userdata + "/data/" ); + BOOST_CHECK_EQUAL(get_wml_lcation("~addons"), userdata + "/data/addons"); - // Inexistent paths are resolved empty. + BOOST_CHECK_EQUAL(get_wml_location(".", get_wml_location("core/units")), gamedata + "/data/core/units"); + BOOST_CHECK_EQUAL(get_wml_location("./bats", get_wml_location("core/units")), gamedata + "/data/core/units/bats"); + BOOST_CHECK_EQUAL(get_wml_location("../macros/ai.cfg", get_wml_location("core/units")), gamedata + "data/core/macros/ai.cfg"); + + // Nonexistent paths are resolved empty. BOOST_CHECK( get_wml_location("why_would_anyone_ever_name_a_file_like_this").empty() ); + + // Unsafe paths are resolved empty. + BOOST_CHECK(get_wml_location("..").empty()); + BOOST_CHECK(get_wml_location("core/../..").empty()); + BOOST_CHECK(get_wml_location("../..", get_wml_location("core")).empty()); } BOOST_AUTO_TEST_CASE( test_fs_search )