diff options
authorwm4 <wm4@nowhere>2020-02-05 16:20:37 +0100
committerwm4 <wm4@nowhere>2020-02-06 11:59:24 +0100
commitcce7062a8a6b6a3b3666aea3ff86db879cba67b6 (patch)
parent65cd9efa850faaedb9f866632feb9fee2ef71462 (diff)
lua: fix highly security relevant arbitrary code execution buglua-remote-code-execution
It appears Lua's package paths try to load .lua files from the current working directory. Not only that, but also shared libraries. WHAT THE FUCK IS WHOEVER IS RESPONSIBLE FOR THIS FUCKING DOING? mpv isn't setting this package path; currently it's only extending it. In any sane world, this wouldn't be a default. Most programs use essentially random working directories and don't change it. I cannot comprehend what bullshit about "convenience" or whatever made them do something this broken and dangerous. Thousands of programs using Lua out there will try to randomly load random code from random directories. In mpv's case, this is so security relevant, because mpv is normally used from the command line, and you will most likely actually change into your media directory or whatever with the shell, and play a file from there. No, you don't want to load a (probably downloaded) shared library from this directory if a script try to load a system lib with the same name or so. I'm not sure why LUA_PATH_DEFAULT in luaconf.h (both upstream and the Debian version) put "./?.lua" at the end, but in any case, trying to load a module that doesn't exist nicely lists all package paths in order, and confirms it tries to load files from the working directory first (anyone can try this). Even if it didn't, this would be problematic at best. Note that scripts are _not_ sandboxed. They're allowed to load system libraries, which is also why we want to keep the non-idiotic parts of the package paths. Attempt to fix this by filtering out relative paths. This is a bit fragile and not very great for something security related, but probably the best we can do without having to make assumptions about the target system file system layout. Also, someone else can fix this for Windows. Also replace ":" with ";" (for the extra path). On a side note, this extra path addition is just in this function out of laziness, since I'd rather not have 2 functions with edit the package path. mpv in default configuration (i.e. no external scripts) is probably not affected. All builtin scripts only "require" preloaded modules, which, in a stroke of genius by the Lua developers, are highest priority in the load order. Otherwise, enjoy your semi-remote code execution bug. Completely unrelated this, I'm open for scripting languages and especially implementations which are all around better than Lua, and are suited for low footprint embedding.
1 files changed, 28 insertions, 14 deletions
diff --git a/player/lua.c b/player/lua.c
index 521a76b4f9..3037fe864c 100644
--- a/player/lua.c
+++ b/player/lua.c
@@ -274,25 +274,38 @@ static int load_scripts(lua_State *L)
return 0;
-static void set_path(lua_State *L)
+static void fuck_lua(lua_State *L, const char *search_path, const char *extra)
- struct script_ctx *ctx = get_ctx(L);
- if (!ctx->path)
- return;
void *tmp = talloc_new(NULL);
lua_getglobal(L, "package"); // package
- lua_getfield(L, -1, "path"); // package path
- const char *path = lua_tostring(L, -1);
+ lua_getfield(L, -1, search_path); // package search_path
+ bstr path = bstr0(lua_tostring(L, -1));
+ char *newpath = talloc_strdup(tmp, "");
+ // Script-directory paths take priority.
+ if (extra) {
+ newpath = talloc_asprintf_append(newpath, "%s%s",
+ newpath[0] ? ";" : "",
+ mp_path_join(tmp, extra, "?.lua"));
+ }
- char *newpath = talloc_asprintf(tmp, "%s;%s",
- mp_path_join(tmp, ctx->path, "?.lua"),
- path ? path : "");
+ // Unbelievable but true: Lua loads .lua files AND dynamic libraries from
+ // the working directory. This is highly security relevant.
+ // Lua scripts are still supposed to load globally installed libraries, so
+ // try to get by by filtering out any relative paths.
+ while (path.len) {
+ bstr item;
+ bstr_split_tok(path, ";", &item, &path);
+ if (bstr_startswith0(item, "/")) {
+ newpath = talloc_asprintf_append(newpath, "%s%.*s",
+ newpath[0] ? ";" : "",
+ BSTR_P(item));
+ }
+ }
- lua_pushstring(L, newpath); // package path newpath
- lua_setfield(L, -3, "path"); // package path
+ lua_pushstring(L, newpath); // package search_path newpath
+ lua_setfield(L, -3, search_path); // package search_path
lua_pop(L, 2); // -
@@ -351,7 +364,8 @@ static int run_lua(lua_State *L)
assert(lua_gettop(L) == 0);
- set_path(L);
+ fuck_lua(L, "path", ctx->path);
+ fuck_lua(L, "cpath", NULL);
assert(lua_gettop(L) == 0);
// run this under an error handler that can do backtraces