Implement script sandboxing for main menu

builtin/mainmenu/async_event.lua

@@ -11,11 +11,6 @@ end
 core.async_event_handler = handle_job
 
 function core.handle_async(func, parameter, callback)
-	-- Serialize function
-	local serialized_func = string.dump(func)
-
-	assert(serialized_func ~= nil)
-
 	-- Serialize parameters
 	local serialized_param = core.serialize(parameter)
 
@@ -23,7 +18,7 @@ function core.handle_async(func, parameter, callback)
 		return false
 	end
 
-	local jobid = core.do_async_callback(serialized_func, serialized_param)
+	local jobid = core.do_async_callback(func, serialized_param)
 
 	core.async_jobs[jobid] = callback
 

builtin/mainmenu/content/contentdb.lua

@@ -392,7 +392,7 @@ function contentdb.resolve_dependencies(package, game, callback)
 end
 
 
-local function fetch_pkgs(params)
+local function fetch_pkgs()
 	local version = core.get_version()
 	local base_url = core.settings:get("contentdb_url")
 	local url = base_url ..
@@ -429,41 +429,43 @@ local function fetch_pkgs(params)
 	if not packages or #packages == 0 then
 		return
 	end
-	local aliases = {}
+	return packages
+end
+
+
+function contentdb.set_packages_from_api(packages)
+	contentdb.package_by_id = {}
+	contentdb.aliases = {}
 
 	for _, package in pairs(packages) do
-		package.id = params.calculate_package_id(package.type, package.author, package.name)
+		package.id = contentdb.calculate_package_id(package.type, package.author, package.name)
 		package.url_part = core.urlencode(package.author) .. "/" .. core.urlencode(package.name)
 
+		contentdb.package_by_id[package.id] = package
+
 		if package.aliases then
 			for _, alias in ipairs(package.aliases) do
 				-- We currently don't support name changing
 				local suffix = "/" .. package.name
 				if alias:sub(-#suffix) == suffix then
-					aliases[alias:lower()] = package.id
+					contentdb.aliases[alias:lower()] = package.id
 				end
 			end
 		end
 	end
 
-	return { packages = packages, aliases = aliases }
+	contentdb.load_ok = true
+	contentdb.load_error = false
+	contentdb.packages = packages
+	contentdb.packages_full = packages
+	contentdb.packages_full_unordered = packages
 end
 
-
 function contentdb.fetch_pkgs(callback)
 	contentdb.loading = true
-	core.handle_async(fetch_pkgs, { calculate_package_id = contentdb.calculate_package_id  }, function(result)
+	core.handle_async(fetch_pkgs, nil, function(result)
 		if result then
-			contentdb.load_ok = true
-			contentdb.load_error = false
-			contentdb.packages = result.packages
-			contentdb.packages_full = result.packages
-			contentdb.packages_full_unordered = result.packages
-			contentdb.aliases = result.aliases
-
-			for _, package in ipairs(result.packages) do
-				contentdb.package_by_id[package.id] = package
-			end
+			contentdb.set_packages_from_api(result)
 		else
 			contentdb.load_error = true
 		end

builtin/mainmenu/init.lua

@@ -133,4 +133,5 @@ local function init_globals()
 	check_new_version()
 end
 
+assert(os.execute == nil)
 init_globals()

src/script/cpp_api/s_async.cpp

@@ -14,10 +14,14 @@ extern "C" {
 #include "server.h"
 #include "s_async.h"
 #include "log.h"
+#include "config.h"
 #include "filesys.h"
 #include "porting.h"
 #include "common/c_internal.h"
 #include "common/c_packer.h"
+#if CHECK_CLIENT_BUILD()
+#include "script/scripting_mainmenu.h"
+#endif
 #include "lua_api/l_base.h"
 
 /******************************************************************************/
@@ -256,7 +260,6 @@ bool AsyncEngine::prepareEnvironment(lua_State* L, int top)
 	return true;
 }
 
-/******************************************************************************/
 AsyncWorkerThread::AsyncWorkerThread(AsyncEngine* jobDispatcher,
 		const std::string &name) :
 	ScriptApiBase(ScriptingType::Async),
@@ -270,6 +273,8 @@ AsyncWorkerThread::AsyncWorkerThread(AsyncEngine* jobDispatcher,
 
 		if (g_settings->getBool("secure.enable_security"))
 			initializeSecurity();
+	} else {
+		initializeSecurity();
 	}
 
 	// Prepare job lua environment
@@ -287,13 +292,27 @@ AsyncWorkerThread::AsyncWorkerThread(AsyncEngine* jobDispatcher,
 	lua_pop(L, 1);
 }
 
-/******************************************************************************/
 AsyncWorkerThread::~AsyncWorkerThread()
 {
 	sanity_check(!isRunning());
 }
 
-/******************************************************************************/
+bool AsyncWorkerThread::checkPathInternal(const std::string &abs_path,
+	bool write_required, bool *write_allowed)
+{
+	auto *L = getStack();
+	// dispatch to the right implementation. this should be refactored some day...
+	if (jobDispatcher->server) {
+		return ScriptApiSecurity::checkPathWithGamedef(L, abs_path, write_required, write_allowed);
+	} else {
+#if CHECK_CLIENT_BUILD()
+		return MainMenuScripting::checkPathAccess(abs_path, write_required, write_allowed);
+#else
+		FATAL_ERROR("should never get here");
+#endif
+	}
+}
+
 void* AsyncWorkerThread::run()
 {
 	if (isErrored)

src/script/cpp_api/s_async.h

@@ -56,10 +56,7 @@ protected:
 	AsyncWorkerThread(AsyncEngine* jobDispatcher, const std::string &name);
 
 	bool checkPathInternal(const std::string &abs_path, bool write_required,
-		bool *write_allowed) override {
-		return ScriptApiSecurity::checkPathWithGamedef(getStack(),
-			abs_path, write_required, write_allowed);
-	};
+		bool *write_allowed) override;
 
 private:
 	AsyncEngine *jobDispatcher = nullptr;

src/script/cpp_api/s_security.cpp

@@ -66,6 +66,7 @@ void ScriptApiSecurity::initializeSecurity()
 		"core",
 		"collectgarbage",
 		"DIR_DELIM",
+		"PLATFORM",
 		"error",
 		"getfenv",
 		"getmetatable",

src/script/lua_api/l_mainmenu.cpp

@@ -343,6 +343,8 @@ int ModApiMainMenu::l_get_content_info(lua_State *L)
 {
 	std::string path = luaL_checkstring(L, 1);
 
+	CHECK_SECURE_PATH(L, path.c_str(), false)
+
 	ContentSpec spec;
 	spec.path = path;
 	parseContentInfo(spec);
@@ -410,6 +412,8 @@ int ModApiMainMenu::l_check_mod_configuration(lua_State *L)
 {
 	std::string worldpath = luaL_checkstring(L, 1);
 
+	CHECK_SECURE_PATH(L, worldpath.c_str(), false)
+
 	ModConfiguration modmgr;
 
 	// Add all game mods
@@ -732,15 +736,13 @@ int ModApiMainMenu::l_get_temp_path(lua_State *L)
 }
 
 /******************************************************************************/
-int ModApiMainMenu::l_create_dir(lua_State *L) {
+int ModApiMainMenu::l_create_dir(lua_State *L)
+{
 	const char *path = luaL_checkstring(L, 1);
 
-	if (ModApiMainMenu::mayModifyPath(path)) {
-		lua_pushboolean(L, fs::CreateAllDirs(path));
-		return 1;
-	}
+	CHECK_SECURE_PATH(L, path, true)
 
-	lua_pushboolean(L, false);
+	lua_pushboolean(L, fs::CreateAllDirs(path));
 	return 1;
 }
 
@@ -749,14 +751,9 @@ int ModApiMainMenu::l_delete_dir(lua_State *L)
 {
 	const char *path = luaL_checkstring(L, 1);
 
-	std::string absolute_path = fs::RemoveRelativePathComponents(path);
+	CHECK_SECURE_PATH(L, path, true)
 
-	if (ModApiMainMenu::mayModifyPath(absolute_path)) {
-		lua_pushboolean(L, fs::RecursiveDelete(absolute_path));
-		return 1;
-	}
-
-	lua_pushboolean(L, false);
+	lua_pushboolean(L, fs::RecursiveDelete(path));
 	return 1;
 }
 
@@ -766,24 +763,16 @@ int ModApiMainMenu::l_copy_dir(lua_State *L)
 	const char *source	= luaL_checkstring(L, 1);
 	const char *destination	= luaL_checkstring(L, 2);
 
-	bool keep_source = true;
-	if (!lua_isnoneornil(L, 3))
-		keep_source = readParam<bool>(L, 3);
+	bool keep_source = readParam<bool>(L, 3, true);
 
-	std::string abs_destination = fs::RemoveRelativePathComponents(destination);
-	std::string abs_source = fs::RemoveRelativePathComponents(source);
-
-	if (!ModApiMainMenu::mayModifyPath(abs_destination) ||
-		(!keep_source && !ModApiMainMenu::mayModifyPath(abs_source))) {
-		lua_pushboolean(L, false);
-		return 1;
-	}
+	CHECK_SECURE_PATH(L, source, !keep_source)
+	CHECK_SECURE_PATH(L, destination, true)
 
 	bool retval;
 	if (keep_source)
-		retval = fs::CopyDir(abs_source, abs_destination);
+		retval = fs::CopyDir(source, destination);
 	else
-		retval = fs::MoveDir(abs_source, abs_destination);
+		retval = fs::MoveDir(source, destination);
 	lua_pushboolean(L, retval);
 	return 1;
 }
@@ -793,6 +782,8 @@ int ModApiMainMenu::l_is_dir(lua_State *L)
 {
 	const char *path = luaL_checkstring(L, 1);
 
+	CHECK_SECURE_PATH(L, path, false)
+
 	lua_pushboolean(L, fs::IsDir(path));
 	return 1;
 }
@@ -803,16 +794,12 @@ int ModApiMainMenu::l_extract_zip(lua_State *L)
 	const char *zipfile	= luaL_checkstring(L, 1);
 	const char *destination	= luaL_checkstring(L, 2);
 
-	std::string absolute_destination = fs::RemoveRelativePathComponents(destination);
+	CHECK_SECURE_PATH(L, zipfile, false)
+	CHECK_SECURE_PATH(L, destination, true)
 
-	if (ModApiMainMenu::mayModifyPath(absolute_destination)) {
-		auto fs = RenderingEngine::get_raw_device()->getFileSystem();
-		bool ok = fs::extractZipFile(fs, zipfile, destination);
-		lua_pushboolean(L, ok);
-		return 1;
-	}
-
-	lua_pushboolean(L,false);
+	auto fs = RenderingEngine::get_raw_device()->getFileSystem();
+	bool ok = fs::extractZipFile(fs, zipfile, destination);
+	lua_pushboolean(L, ok);
 	return 1;
 }
 
@@ -826,40 +813,13 @@ int ModApiMainMenu::l_get_mainmenu_path(lua_State *L)
 	return 1;
 }
 
-/******************************************************************************/
-bool ModApiMainMenu::mayModifyPath(std::string path)
-{
-	path = fs::RemoveRelativePathComponents(path);
-
-	if (fs::PathStartsWith(path, fs::TempPath()))
-		return true;
-
-	std::string path_user = fs::RemoveRelativePathComponents(porting::path_user);
-
-	if (fs::PathStartsWith(path, path_user + DIR_DELIM "client"))
-		return true;
-	if (fs::PathStartsWith(path, path_user + DIR_DELIM "games"))
-		return true;
-	if (fs::PathStartsWith(path, path_user + DIR_DELIM "mods"))
-		return true;
-	if (fs::PathStartsWith(path, path_user + DIR_DELIM "textures"))
-		return true;
-	if (fs::PathStartsWith(path, path_user + DIR_DELIM "worlds"))
-		return true;
-
-	if (fs::PathStartsWith(path, fs::RemoveRelativePathComponents(porting::path_cache)))
-		return true;
-
-	return false;
-}
-
-
 /******************************************************************************/
 int ModApiMainMenu::l_may_modify_path(lua_State *L)
 {
 	const char *target = luaL_checkstring(L, 1);
-	std::string absolute_destination = fs::RemoveRelativePathComponents(target);
-	lua_pushboolean(L, ModApiMainMenu::mayModifyPath(absolute_destination));
+	bool write_allowed = false;
+	bool ok = ScriptApiSecurity::checkPath(L, target, false, &write_allowed);
+	lua_pushboolean(L, ok && write_allowed);
 	return 1;
 }
 
@@ -892,19 +852,9 @@ int ModApiMainMenu::l_download_file(lua_State *L)
 	const char *url    = luaL_checkstring(L, 1);
 	const char *target = luaL_checkstring(L, 2);
 
-	//check path
-	std::string absolute_destination = fs::RemoveRelativePathComponents(target);
+	CHECK_SECURE_PATH(L, target, true)
 
-	if (ModApiMainMenu::mayModifyPath(absolute_destination)) {
-		if (GUIEngine::downloadFile(url,absolute_destination)) {
-			lua_pushboolean(L,true);
-			return 1;
-		}
-	} else {
-		errorstream << "DOWNLOAD denied: " << absolute_destination
-				<< " isn't an allowed path" << std::endl;
-	}
-	lua_pushboolean(L,false);
+	lua_pushboolean(L, GUIEngine::downloadFile(url, target));
 	return 1;
 }
 
@@ -1068,16 +1018,22 @@ int ModApiMainMenu::l_open_url_dialog(lua_State *L)
 /******************************************************************************/
 int ModApiMainMenu::l_open_dir(lua_State *L)
 {
-	std::string path = luaL_checkstring(L, 1);
-	lua_pushboolean(L, porting::open_directory(path));
+	const char *target = luaL_checkstring(L, 1);
+
+	CHECK_SECURE_PATH(L, target, false)
+
+	lua_pushboolean(L, porting::open_directory(target));
 	return 1;
 }
 
 /******************************************************************************/
 int ModApiMainMenu::l_share_file(lua_State *L)
 {
+	const char *path = luaL_checkstring(L, 1);
+
+	CHECK_SECURE_PATH(L, path, false)
+
 #ifdef __ANDROID__
-	std::string path = luaL_checkstring(L, 1);
 	porting::shareFileAndroid(path);
 	lua_pushboolean(L, true);
 #else
@@ -1091,19 +1047,20 @@ int ModApiMainMenu::l_do_async_callback(lua_State *L)
 {
 	MainMenuScripting *script = getScriptApi<MainMenuScripting>(L);
 
-	size_t func_length, param_length;
-	const char* serialized_func_raw = luaL_checklstring(L, 1, &func_length);
-	const char* serialized_param_raw = luaL_checklstring(L, 2, &param_length);
+	luaL_checktype(L, 1, LUA_TFUNCTION);
+	call_string_dump(L, 1);
+	size_t func_length;
+	const char *serialized_func_raw = lua_tolstring(L, -1, &func_length);
 
-	sanity_check(serialized_func_raw != NULL);
-	sanity_check(serialized_param_raw != NULL);
+	size_t param_length;
+	const char* serialized_param_raw = luaL_checklstring(L, 2, &param_length);
 
 	u32 jobId = script->queueAsync(
 		std::string(serialized_func_raw, func_length),
 		std::string(serialized_param_raw, param_length));
 
+	lua_settop(L, 0);
 	lua_pushinteger(L, jobId);
-
 	return 1;
 }
 

src/script/lua_api/l_mainmenu.h

@@ -37,14 +37,6 @@ private:
 	 */
 	static int getBoolData(lua_State *L, const std::string &name ,bool& valid);
 
-	/**
-	 * Checks if a path may be modified. Paths in the temp directory or the user
-	 * games, mods, textures, or worlds directories may be modified.
-	 * @param path path to check
-	 * @return true if the path may be modified
-	 */
-	static bool mayModifyPath(std::string path);
-
 	//api calls
 
 	static int l_start(lua_State *L);

src/script/scripting_mainmenu.cpp

@@ -12,11 +12,14 @@
 #include "lua_api/l_util.h"
 #include "lua_api/l_settings.h"
 #include "log.h"
+#include "filesys.h"
+#include "porting.h"
 
 extern "C" {
 #include "lualib.h"
 }
-#define MAINMENU_NUM_ASYNC_THREADS 4
+
+#define MAINMENU_NUM_ASYNC_THREADS 2
 
 
 MainMenuScripting::MainMenuScripting(GUIEngine* guiengine):
@@ -26,6 +29,8 @@ MainMenuScripting::MainMenuScripting(GUIEngine* guiengine):
 
 	SCRIPTAPI_PRECHECKHEADER
 
+	initializeSecurity();
+
 	lua_getglobal(L, "core");
 	int top = lua_gettop(L);
 
@@ -69,6 +74,42 @@ void MainMenuScripting::registerLuaClasses(lua_State *L, int top)
 	MainMenuSoundHandle::Register(L);
 }
 
+bool MainMenuScripting::mayModifyPath(const std::string &path)
+{
+	if (fs::PathStartsWith(path, fs::TempPath()))
+		return true;
+
+	std::string path_user = fs::RemoveRelativePathComponents(porting::path_user);
+
+	if (fs::PathStartsWith(path, path_user + DIR_DELIM "client"))
+		return true;
+	if (fs::PathStartsWith(path, path_user + DIR_DELIM "games"))
+		return true;
+	if (fs::PathStartsWith(path, path_user + DIR_DELIM "mods"))
+		return true;
+	if (fs::PathStartsWith(path, path_user + DIR_DELIM "textures"))
+		return true;
+	if (fs::PathStartsWith(path, path_user + DIR_DELIM "worlds"))
+		return true;
+
+	if (fs::PathStartsWith(path, fs::RemoveRelativePathComponents(porting::path_cache)))
+		return true;
+
+	return false;
+}
+
+bool MainMenuScripting::checkPathAccess(const std::string &abs_path, bool write_required,
+	bool *write_allowed)
+{
+	if (mayModifyPath(abs_path)) {
+		if (write_allowed)
+			*write_allowed = true;
+		return true;
+	}
+	// TODO?: global read access sounds too broad
+	return !write_required;
+}
+
 void MainMenuScripting::beforeClose()
 {
 	SCRIPTAPI_PRECHECKHEADER

src/script/scripting_mainmenu.h

@@ -6,6 +6,7 @@
 
 #include "cpp_api/s_base.h"
 #include "cpp_api/s_mainmenu.h"
+#include "cpp_api/s_security.h"
 #include "cpp_api/s_async.h"
 
 /*****************************************************************************/
@@ -14,7 +15,8 @@
 
 class MainMenuScripting
 		: virtual public ScriptApiBase,
-		  public ScriptApiMainMenu
+		  public ScriptApiMainMenu,
+		  public ScriptApiSecurity
 {
 public:
 	MainMenuScripting(GUIEngine* guiengine);
@@ -29,6 +31,19 @@ public:
 	u32 queueAsync(std::string &&serialized_func,
 		std::string &&serialized_param);
 
+	// Is the main menu allowed writeable access to this path?
+	static bool mayModifyPath(const std::string &path);
+
+	// (public implementation so it can be used from AsyncEngine)
+	static bool checkPathAccess(const std::string &abs_path, bool write_required,
+		bool *write_allowed);
+
+protected:
+	bool checkPathInternal(const std::string &abs_path, bool write_required,
+		bool *write_allowed) override {
+		return checkPathAccess(abs_path, write_required, write_allowed);
+	}
+
 private:
 	void initializeModApi(lua_State *L, int top);
 	static void registerLuaClasses(lua_State *L, int top);