Skip to content

Commit 3a8c788

Browse files
ShadowNinjaShadowNinja
committedMay 16, 2015
Add mod security
Due to compatibility concerns, this is temporarily disabled.
·
5.11.00.4.13
1 parent f264212 commit 3a8c788

22 files changed

+809
-80
lines changed
 

Diff for: ‎build/android/jni/Android.mk

+2-1
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,7 @@ LOCAL_SRC_FILES += \
262262
jni/src/script/common/c_converter.cpp \
263263
jni/src/script/common/c_internal.cpp \
264264
jni/src/script/common/c_types.cpp \
265+
jni/src/script/cpp_api/s_async.cpp \
265266
jni/src/script/cpp_api/s_base.cpp \
266267
jni/src/script/cpp_api/s_entity.cpp \
267268
jni/src/script/cpp_api/s_env.cpp \
@@ -271,8 +272,8 @@ LOCAL_SRC_FILES += \
271272
jni/src/script/cpp_api/s_node.cpp \
272273
jni/src/script/cpp_api/s_nodemeta.cpp \
273274
jni/src/script/cpp_api/s_player.cpp \
275+
jni/src/script/cpp_api/s_security.cpp \
274276
jni/src/script/cpp_api/s_server.cpp \
275-
jni/src/script/cpp_api/s_async.cpp \
276277
jni/src/script/lua_api/l_base.cpp \
277278
jni/src/script/lua_api/l_craft.cpp \
278279
jni/src/script/lua_api/l_env.cpp \

Diff for: ‎doc/lua_api.txt

+4
Original file line numberDiff line numberDiff line change
@@ -1825,8 +1825,12 @@ Call these functions only at load time!
18251825

18261826
### Setting-related
18271827
* `minetest.setting_set(name, value)`
1828+
* Setting names can't contain whitespace or any of `="{}#`.
1829+
* Setting values can't contain the sequence `\n"""`.
1830+
* Setting names starting with "secure." can't be set.
18281831
* `minetest.setting_get(name)`: returns string or `nil`
18291832
* `minetest.setting_setbool(name, value)`
1833+
* See documentation on `setting_set` for restrictions.
18301834
* `minetest.setting_getbool(name)`: returns boolean or `nil`
18311835
* `minetest.setting_get_pos(name)`: returns position or nil
18321836
* `minetest.setting_save()`, returns `nil`, save all settings to config file

Diff for: ‎minetest.conf.example

+3
Original file line numberDiff line numberDiff line change
@@ -569,3 +569,6 @@
569569
#mgv7_np_cave1 = 0, 12, (100, 100, 100), 52534, 4, 0.5, 2.0
570570
#mgv7_np_cave2 = 0, 12, (100, 100, 100), 10325, 4, 0.5, 2.0
571571

572+
# Prevent mods from doing insecure things like running shell commands.
573+
#secure.enable_security = false
574+

Diff for: ‎src/defaultsettings.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ void set_default_settings(Settings *settings)
272272
settings->setDefault("emergequeue_limit_diskonly", "32");
273273
settings->setDefault("emergequeue_limit_generate", "32");
274274
settings->setDefault("num_emerge_threads", "1");
275+
settings->setDefault("secure.enable_security", "false");
275276

276277
// physics stuff
277278
settings->setDefault("movement_acceleration_default", "3");

Diff for: ‎src/filesys.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,19 @@ std::string RemoveRelativePathComponents(std::string path)
662662
return path.substr(0, pos);
663663
}
664664

665+
std::string AbsolutePath(const std::string &path)
666+
{
667+
#ifdef _WIN32
668+
char *abs_path = _fullpath(NULL, path.c_str(), MAX_PATH);
669+
#else
670+
char *abs_path = realpath(path.c_str(), NULL);
671+
#endif
672+
if (!abs_path) return "";
673+
std::string abs_path_str(abs_path);
674+
free(abs_path);
675+
return abs_path_str;
676+
}
677+
665678
const char *GetFilenameFromPath(const char *path)
666679
{
667680
const char *filename = strrchr(path, DIR_DELIM_CHAR);

Diff for: ‎src/filesys.h

+7-3
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,17 @@ std::string RemoveLastPathComponent(const std::string &path,
103103
// this does not resolve symlinks and check for existence of directories.
104104
std::string RemoveRelativePathComponents(std::string path);
105105

106-
// Return the filename from a path or the entire path if no directory delimiter
107-
// is found.
106+
// Returns the absolute path for the passed path, with "." and ".." path
107+
// components and symlinks removed. Returns "" on error.
108+
std::string AbsolutePath(const std::string &path);
109+
110+
// Returns the filename from a path or the entire path if no directory
111+
// delimiter is found.
108112
const char *GetFilenameFromPath(const char *path);
109113

110114
bool safeWriteToFile(const std::string &path, const std::string &content);
111115

112-
}//fs
116+
} // namespace fs
113117

114118
#endif
115119

Diff for: ‎src/script/common/c_internal.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,8 @@ int script_exception_wrapper(lua_State *L, lua_CFunction f)
6363
return f(L); // Call wrapped function and return result.
6464
} catch (const char *s) { // Catch and convert exceptions.
6565
lua_pushstring(L, s);
66-
} catch (std::exception& e) {
66+
} catch (std::exception &e) {
6767
lua_pushstring(L, e.what());
68-
} catch (...) {
69-
lua_pushliteral(L, "caught (...)");
7068
}
7169
return lua_error(L); // Rethrow as a Lua error.
7270
}

Diff for: ‎src/script/cpp_api/CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
set(common_SCRIPT_CPP_API_SRCS
2+
${CMAKE_CURRENT_SOURCE_DIR}/s_async.cpp
23
${CMAKE_CURRENT_SOURCE_DIR}/s_base.cpp
34
${CMAKE_CURRENT_SOURCE_DIR}/s_entity.cpp
45
${CMAKE_CURRENT_SOURCE_DIR}/s_env.cpp
@@ -7,8 +8,8 @@ set(common_SCRIPT_CPP_API_SRCS
78
${CMAKE_CURRENT_SOURCE_DIR}/s_node.cpp
89
${CMAKE_CURRENT_SOURCE_DIR}/s_nodemeta.cpp
910
${CMAKE_CURRENT_SOURCE_DIR}/s_player.cpp
11+
${CMAKE_CURRENT_SOURCE_DIR}/s_security.cpp
1012
${CMAKE_CURRENT_SOURCE_DIR}/s_server.cpp
11-
${CMAKE_CURRENT_SOURCE_DIR}/s_async.cpp
1213
PARENT_SCOPE)
1314

1415
set(client_SCRIPT_CPP_API_SRCS

Diff for: ‎src/script/cpp_api/s_base.cpp

+22-22
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
1919

2020
#include "cpp_api/s_base.h"
2121
#include "cpp_api/s_internal.h"
22+
#include "cpp_api/s_security.h"
2223
#include "lua_api/l_object.h"
2324
#include "serverobject.h"
2425
#include "debug.h"
@@ -45,18 +46,18 @@ class ModNameStorer
4546
private:
4647
lua_State *L;
4748
public:
48-
ModNameStorer(lua_State *L_, const std::string &modname):
49+
ModNameStorer(lua_State *L_, const std::string &mod_name):
4950
L(L_)
5051
{
51-
// Store current modname in registry
52-
lua_pushstring(L, modname.c_str());
53-
lua_setfield(L, LUA_REGISTRYINDEX, "current_modname");
52+
// Store current mod name in registry
53+
lua_pushstring(L, mod_name.c_str());
54+
lua_setfield(L, LUA_REGISTRYINDEX, SCRIPT_MOD_NAME_FIELD);
5455
}
5556
~ModNameStorer()
5657
{
57-
// Clear current modname in registry
58+
// Clear current mod name from registry
5859
lua_pushnil(L);
59-
lua_setfield(L, LUA_REGISTRYINDEX, "current_modname");
60+
lua_setfield(L, LUA_REGISTRYINDEX, SCRIPT_MOD_NAME_FIELD);
6061
}
6162
};
6263

@@ -112,32 +113,31 @@ ScriptApiBase::~ScriptApiBase()
112113
lua_close(m_luastack);
113114
}
114115

115-
bool ScriptApiBase::loadMod(const std::string &scriptpath,
116-
const std::string &modname)
116+
bool ScriptApiBase::loadMod(const std::string &script_path,
117+
const std::string &mod_name)
117118
{
118-
ModNameStorer modnamestorer(getStack(), modname);
119+
ModNameStorer mod_name_storer(getStack(), mod_name);
119120

120-
if (!string_allowed(modname, MODNAME_ALLOWED_CHARS)) {
121-
errorstream<<"Error loading mod \""<<modname
122-
<<"\": modname does not follow naming conventions: "
123-
<<"Only chararacters [a-z0-9_] are allowed."<<std::endl;
124-
return false;
125-
}
126-
127-
return loadScript(scriptpath);
121+
return loadScript(script_path);
128122
}
129123

130-
bool ScriptApiBase::loadScript(const std::string &scriptpath)
124+
bool ScriptApiBase::loadScript(const std::string &script_path)
131125
{
132-
verbosestream<<"Loading and running script from "<<scriptpath<<std::endl;
126+
verbosestream << "Loading and running script from " << script_path << std::endl;
133127

134128
lua_State *L = getStack();
135129

136-
int ret = luaL_loadfile(L, scriptpath.c_str()) || lua_pcall(L, 0, 0, m_errorhandler);
137-
if (ret) {
130+
bool ok;
131+
if (m_secure) {
132+
ok = ScriptApiSecurity::safeLoadFile(L, script_path.c_str());
133+
} else {
134+
ok = !luaL_loadfile(L, script_path.c_str());
135+
}
136+
ok = ok && !lua_pcall(L, 0, 0, m_errorhandler);
137+
if (!ok) {
138138
errorstream << "========== ERROR FROM LUA ===========" << std::endl;
139139
errorstream << "Failed to load and run script from " << std::endl;
140-
errorstream << scriptpath << ":" << std::endl;
140+
errorstream << script_path << ":" << std::endl;
141141
errorstream << std::endl;
142142
errorstream << lua_tostring(L, -1) << std::endl;
143143
errorstream << std::endl;

Diff for: ‎src/script/cpp_api/s_base.h

+11-4
Original file line numberDiff line numberDiff line change
@@ -35,24 +35,31 @@ extern "C" {
3535

3636
#define SCRIPTAPI_LOCK_DEBUG
3737

38+
#define SCRIPT_MOD_NAME_FIELD "current_mod_name"
39+
// MUST be an invalid mod name so that mods can't
40+
// use that name to bypass security!
41+
#define BUILTIN_MOD_NAME "*builtin*"
42+
43+
3844
class Server;
3945
class Environment;
4046
class GUIEngine;
4147
class ServerActiveObject;
4248

4349
class ScriptApiBase {
4450
public:
45-
4651
ScriptApiBase();
4752
virtual ~ScriptApiBase();
4853

49-
bool loadMod(const std::string &scriptpath, const std::string &modname);
50-
bool loadScript(const std::string &scriptpath);
54+
bool loadMod(const std::string &script_path, const std::string &mod_name);
55+
bool loadScript(const std::string &script_path);
5156

5257
/* object */
5358
void addObjectReference(ServerActiveObject *cobj);
5459
void removeObjectReference(ServerActiveObject *cobj);
5560

61+
Server* getServer() { return m_server; }
62+
5663
protected:
5764
friend class LuaABM;
5865
friend class InvRef;
@@ -69,7 +76,6 @@ class ScriptApiBase {
6976
void scriptError();
7077
void stackDump(std::ostream &o);
7178

72-
Server* getServer() { return m_server; }
7379
void setServer(Server* server) { m_server = server; }
7480

7581
Environment* getEnv() { return m_environment; }
@@ -84,6 +90,7 @@ class ScriptApiBase {
8490
JMutex m_luastackmutex;
8591
// Stack index of Lua error handler
8692
int m_errorhandler;
93+
bool m_secure;
8794
#ifdef SCRIPTAPI_LOCK_DEBUG
8895
bool m_locked;
8996
#endif

0 commit comments

Comments
 (0)