Skip to content

Commit

Permalink
Accept ".." in file paths everywhere
Browse files Browse the repository at this point in the history 
This accepts ".." without escaping the sandbox by first resolving it and
failing if the result begins in "..". Only then is the relative path
resolved to an absolute path.
  • Loading branch information
@CelticMinstrel
CelticMinstrel committed Oct 24, 2018
1 parent fa12518 commit 165f2cd
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 72 deletions.
89 changes: 69 additions & 20 deletions src/filesystem.cpp
View file
Expand Up @@ -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";
Expand All @@ -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;
}

Expand Down Expand Up @@ -1282,20 +1333,9 @@ const std::vector<std::string>& 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();
Expand All @@ -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();
}

Expand All @@ -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 &current_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] == '~') {
Expand Down
6 changes: 3 additions & 3 deletions src/game_config_manager.cpp
View file
Expand Up @@ -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
Expand Down
47 changes: 3 additions & 44 deletions src/scripting/lua_fileops.cpp
View file
Expand Up @@ -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;
}

Expand Down
29 changes: 24 additions & 5 deletions src/tests/test_filesystem.cpp
View file
Expand Up @@ -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" );

Expand All @@ -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 )
Expand All @@ -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 )
Expand Down

0 comments on commit 165f2cd

Please sign in to comment.