RepoMirrors/mpv
1
0
Fork 0
mirror of https://github.com/mpv-player/mpv synced 2024-12-29 02:22:19 +00:00
Code Issues Releases Wiki Activity

lua: fix highly security relevant arbitrary code execution bug

Browse Source 
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.
This commit is contained in:
wm4 2020-02-05 16:20:37 +01:00
parent 65cd9efa85
commit cce7062a8a
1 changed files with 28 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
player/lua.c
View File

@ -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, "");
char *newpath = talloc_asprintf(tmp, "%s;%s",
mp_path_join(tmp, ctx->path, "?.lua"),
path ? path : "");
// Script-directory paths take priority.
if (extra) {
newpath = talloc_asprintf_append(newpath, "%s%s",
newpath[0] ? ";" : "",
mp_path_join(tmp, extra, "?.lua"));
}
lua_pushstring(L, newpath); // package path newpath
lua_setfield(L, -3, "path"); // package 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 search_path newpath
lua_setfield(L, -3, search_path); // package search_path
lua_pop(L, 2); // -
talloc_free(tmp);
@ -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