Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[no squash] Remove insecure environment from async and emerge environment #14370

Merged
merged 2 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions doc/lua_api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6572,7 +6572,6 @@ Class instances that can be transferred between environments:
Functions:
* Standalone helpers such as logging, filesystem, encoding,
hashing or compression APIs
* `minetest.request_insecure_environment` (same restrictions apply)

Variables:
* `minetest.settings`
Expand Down Expand Up @@ -6641,7 +6640,6 @@ Classes:
Functions:
* Standalone helpers such as logging, filesystem, encoding,
hashing or compression APIs
* `minetest.request_insecure_environment` (same restrictions apply)
* `minetest.get_biome_id`, `get_biome_name`, `get_heat`, `get_humidity`,
`get_biome_data`, `get_mapgen_object`, `get_mapgen_params`, `get_mapgen_edges`,
`get_mapgen_setting`, `get_noiseparams`, `get_decoration_id` and more
Expand Down
41 changes: 41 additions & 0 deletions src/script/cpp_api/s_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ int ScriptApiBase::luaPanic(lua_State *L)
return 0;
}

#ifndef SERVER
void ScriptApiBase::clientOpenLibs(lua_State *L)
{
static const std::vector<std::pair<std::string, lua_CFunction>> m_libs = {
Expand All @@ -199,6 +200,7 @@ void ScriptApiBase::clientOpenLibs(lua_State *L)
lua_call(L, 1, 0);
}
}
#endif

void ScriptApiBase::checkSetByBuiltin()
{
Expand All @@ -223,6 +225,45 @@ void ScriptApiBase::checkSetByBuiltin()
}
}

std::string ScriptApiBase::getCurrentModNameInsecure(lua_State *L)
{
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
auto ret = lua_isstring(L, -1) ? readParam<std::string>(L, -1) : "";
lua_pop(L, 1);
return ret;
}

std::string ScriptApiBase::getCurrentModName(lua_State *L)
{
auto script = ModApiBase::getScriptApiBase(L);
if (script->getType() == ScriptingType::Async ||
script->getType() == ScriptingType::Emerge)
{
// As a precaution never return a "secure" mod name in the async and
// emerge environment, because these currently do not track mod origins
// in a spoof-safe way (see l_register_async_dofile and l_register_mapgen_script).
return "";
}

// We have to make sure that this function is being called directly by
// a mod, otherwise a malicious mod could override a function and
// steal its return value. (e.g. request_insecure_environment)
lua_Debug info;

// Make sure there's only one item below this function on the stack...
if (lua_getstack(L, 2, &info))
return "";
FATAL_ERROR_IF(!lua_getstack(L, 1, &info), "lua_getstack() failed");
FATAL_ERROR_IF(!lua_getinfo(L, "S", &info), "lua_getinfo() failed");

// ...and that that item is the main file scope.
if (strcmp(info.what, "main") != 0)
return "";

// at this point we can trust this value:
return getCurrentModNameInsecure(L);
}

void ScriptApiBase::loadMod(const std::string &script_path,
const std::string &mod_name)
{
Expand Down
20 changes: 18 additions & 2 deletions src/script/cpp_api/s_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,29 @@ class ScriptApiBase : protected LuaHelper {
Client* getClient();
#endif

// IMPORTANT: these cannot be used for any security-related uses, they exist
// only to enrich error messages
// IMPORTANT: These cannot be used for any security-related uses, they exist
// only to enrich error messages.
const std::string &getOrigin() { return m_last_run_mod; }
void setOriginDirect(const char *origin);
void setOriginFromTableRaw(int index, const char *fxn);

// Returns the currently running mod, only during init time.
// The reason this is "insecure" is that mods can mess with each others code,
// so the boundary of who is responsible is fuzzy.
// Note: checking this against BUILTIN_MOD_NAME is always safe (not spoofable).
// returns "" on error
static std::string getCurrentModNameInsecure(lua_State *L);
// Returns the currently running mod, only during init time.
// This checks the Lua stack to only permit direct calls in the file
// scope. That way it is assured that it's really the mod it claims to be.
// returns "" on error
static std::string getCurrentModName(lua_State *L);

#ifdef SERVER
inline void clientOpenLibs(lua_State *L) { assert(false); }
#else
void clientOpenLibs(lua_State *L);
#endif

// Check things that should be set by the builtin mod.
void checkSetByBuiltin();
Expand Down
29 changes: 5 additions & 24 deletions src/script/cpp_api/s_security.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -555,10 +555,9 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path,
return false;

// Get mod name
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
if (lua_isstring(L, -1)) {
std::string mod_name = readParam<std::string>(L, -1);

// FIXME: insecure = problem here?
std::string mod_name = ScriptApiBase::getCurrentModNameInsecure(L);
if (!mod_name.empty()) {
// Builtin can access anything
if (mod_name == BUILTIN_MOD_NAME) {
if (write_allowed) *write_allowed = true;
Expand All @@ -578,7 +577,6 @@ bool ScriptApiSecurity::checkPath(lua_State *L, const char *path,
}
}
}
lua_pop(L, 1); // Pop mod name

// Allow read-only access to game directory
if (!write_required) {
Expand Down Expand Up @@ -629,26 +627,9 @@ bool ScriptApiSecurity::checkWhitelisted(lua_State *L, const std::string &settin
{
assert(str_starts_with(setting, "secure."));

// We have to make sure that this function is being called directly by
// a mod, otherwise a malicious mod could override this function and
// steal its return value.
lua_Debug info;

// Make sure there's only one item below this function on the stack...
if (lua_getstack(L, 2, &info))
return false;
FATAL_ERROR_IF(!lua_getstack(L, 1, &info), "lua_getstack() failed");
FATAL_ERROR_IF(!lua_getinfo(L, "S", &info), "lua_getinfo() failed");

// ...and that that item is the main file scope.
if (strcmp(info.what, "main") != 0)
return false;

// Mod must be listed in secure.http_mods or secure.trusted_mods
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
if (!lua_isstring(L, -1))
std::string mod_name = ScriptApiBase::getCurrentModName(L);
if (mod_name.empty())
return false;
std::string mod_name = readParam<std::string>(L, -1);

std::string value = g_settings->get(setting);
value.erase(std::remove(value.begin(), value.end(), ' '), value.end());
Expand Down
3 changes: 1 addition & 2 deletions src/script/lua_api/l_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ EmergeThread *ModApiBase::getEmergeThread(lua_State *L)

std::string ModApiBase::getCurrentModPath(lua_State *L)
{
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
std::string current_mod_name = readParam<std::string>(L, -1, "");
std::string current_mod_name = ScriptApiBase::getCurrentModNameInsecure(L);
if (current_mod_name.empty())
return ".";

Expand Down
32 changes: 5 additions & 27 deletions src/script/lua_api/l_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ const static CSMFlagDesc flagdesc_csm_restriction[] = {
// get_current_modname()
int ModApiClient::l_get_current_modname(lua_State *L)
{
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
std::string s = ScriptApiBase::getCurrentModNameInsecure(L);
if (!s.empty())
lua_pushstring(L, s.c_str());
else
lua_pushnil(L);
return 1;
}

Expand All @@ -76,30 +80,6 @@ int ModApiClient::l_get_modpath(lua_State *L)
return 1;
}

// get_last_run_mod()
int ModApiClient::l_get_last_run_mod(lua_State *L)
{
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
std::string current_mod = readParam<std::string>(L, -1, "");
if (current_mod.empty()) {
lua_pop(L, 1);
lua_pushstring(L, getScriptApiBase(L)->getOrigin().c_str());
}
return 1;
}

// set_last_run_mod(modname)
int ModApiClient::l_set_last_run_mod(lua_State *L)
{
if (!lua_isstring(L, 1))
return 0;

const char *mod = lua_tostring(L, 1);
getScriptApiBase(L)->setOriginDirect(mod);
lua_pushboolean(L, true);
return 1;
}

// print(text)
int ModApiClient::l_print(lua_State *L)
{
Expand Down Expand Up @@ -367,8 +347,6 @@ void ModApiClient::Initialize(lua_State *L, int top)
API_FCT(send_chat_message);
API_FCT(clear_out_chat_queue);
API_FCT(get_player_names);
API_FCT(set_last_run_mod);
API_FCT(get_last_run_mod);
sfan5 marked this conversation as resolved.
Show resolved Hide resolved
API_FCT(show_formspec);
API_FCT(send_respawn);
API_FCT(gettext);
Expand Down
6 changes: 0 additions & 6 deletions src/script/lua_api/l_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ class ModApiClient : public ModApiBase
// gettext(text)
static int l_gettext(lua_State *L);

// get_last_run_mod(n)
static int l_get_last_run_mod(lua_State *L);

// set_last_run_mod(modname)
static int l_set_last_run_mod(lua_State *L);

// get_node(pos)
static int l_get_node_or_nil(lua_State *L);

Expand Down
22 changes: 11 additions & 11 deletions src/script/lua_api/l_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,11 @@ int ModApiServer::l_show_formspec(lua_State *L)
int ModApiServer::l_get_current_modname(lua_State *L)
{
NO_MAP_LOCK_REQUIRED;
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
std::string s = ScriptApiBase::getCurrentModNameInsecure(L);
if (!s.empty())
lua_pushstring(L, s.c_str());
else
lua_pushnil(L);
return 1;
}

Expand Down Expand Up @@ -656,11 +660,9 @@ int ModApiServer::l_register_async_dofile(lua_State *L)
std::string path = readParam<std::string>(L, 1);
CHECK_SECURE_PATH(L, path.c_str(), false);

// Find currently running mod name (only at init time)
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
if (!lua_isstring(L, -1))
return 0;
std::string modname = readParam<std::string>(L, -1);
std::string modname = ScriptApiBase::getCurrentModNameInsecure(L);
if (modname.empty())
throw ModError("cannot determine mod name");

getServer(L)->m_async_init_files.emplace_back(modname, path);
lua_pushboolean(L, true);
Expand All @@ -675,11 +677,9 @@ int ModApiServer::l_register_mapgen_script(lua_State *L)
std::string path = readParam<std::string>(L, 1);
CHECK_SECURE_PATH(L, path.c_str(), false);

// Find currently running mod name (only at init time)
lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
if (!lua_isstring(L, -1))
return 0;
std::string modname = readParam<std::string>(L, -1);
std::string modname = ScriptApiBase::getCurrentModNameInsecure(L);
if (modname.empty())
throw ModError("cannot determine mod name");

getServer(L)->m_mapgen_init_files.emplace_back(modname, path);
lua_pushboolean(L, true);
Expand Down
15 changes: 8 additions & 7 deletions src/script/lua_api/l_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -632,12 +632,10 @@ int ModApiUtil::l_get_last_run_mod(lua_State *L)
{
NO_MAP_LOCK_REQUIRED;

lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_CURRENT_MOD_NAME);
std::string current_mod = readParam<std::string>(L, -1, "");
if (current_mod.empty()) {
lua_pop(L, 1);
lua_pushstring(L, getScriptApiBase(L)->getOrigin().c_str());
}
std::string current_mod = ScriptApiBase::getCurrentModNameInsecure(L);
if (current_mod.empty())
current_mod = getScriptApiBase(L)->getOrigin();
lua_pushstring(L, current_mod.c_str());
return 1;
}

Expand Down Expand Up @@ -735,6 +733,9 @@ void ModApiUtil::InitializeClient(lua_State *L, int top)
API_FCT(colorspec_to_colorstring);
API_FCT(colorspec_to_bytes);

API_FCT(get_last_run_mod);
API_FCT(set_last_run_mod);

API_FCT(urlencode);

LuaSettings::create(L, g_settings, g_settings_path);
Expand Down Expand Up @@ -765,7 +766,7 @@ void ModApiUtil::InitializeAsync(lua_State *L, int top)
API_FCT(get_dir_list);
API_FCT(safe_file_write);

API_FCT(request_insecure_environment);
// no request_insecure_environment here! mod origins are not tracked securely here.

API_FCT(encode_base64);
API_FCT(decode_base64);
Expand Down