From 5f8c4c6b66e985a2cc6b26f000c1c7af9cfd0ff7 Mon Sep 17 00:00:00 2001 From: Mm2PL Date: Sat, 16 Dec 2023 13:16:54 +0100 Subject: [PATCH] BREAKING: Replace custom `import()` with normal Lua `require()`. (#5014) * Use require() instead of a custom import() * Also search relative to the current file * Update documentation --- CHANGELOG.md | 1 + docs/wip-plugins.md | 21 ++-- src/controllers/plugins/LuaAPI.cpp | 113 +++++++++++-------- src/controllers/plugins/LuaAPI.hpp | 5 +- src/controllers/plugins/PluginController.cpp | 59 +++++++--- src/controllers/plugins/PluginController.hpp | 3 +- 6 files changed, 131 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c44ffde02..44a9f740a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,7 @@ - Dev: Refactored the Image Uploader feature. (#4971) - Dev: Fixed deadlock and use-after-free in tests. (#4981) - Dev: Load less message history upon reconnects. (#5001) +- Dev: BREAKING: Replace custom `import()` with normal Lua `require()`. (#5014) - Dev: Fixed most compiler warnings. (#5028) ## 2.4.6 diff --git a/docs/wip-plugins.md b/docs/wip-plugins.md index 8bb0cf61c..9a3ee25ce 100644 --- a/docs/wip-plugins.md +++ b/docs/wip-plugins.md @@ -158,18 +158,25 @@ It achieves this by forcing all inputs to be encoded with `UTF-8`. See [official documentation](https://www.lua.org/manual/5.4/manual.html#pdf-load) -#### `import(filename)` +#### `require(modname)` -This function mimics Lua's `dofile` however relative paths are relative to your plugin's directory. -You are restricted to loading files in your plugin's directory. You cannot load files with bytecode inside. +This is Lua's [`require()`](https://www.lua.org/manual/5.3/manual.html#pdf-require) function. +However the searcher and load configuration is notably different than default: + +- Lua's built-in dynamic library searcher is removed, +- `package.path` is not used, in its place are two searchers, +- when `require()` is used, first a file relative to the currently executing + file will be checked, then a file relative to the plugin directory, +- binary chunks are never loaded + +As in normal Lua, dots are converted to the path separators (`'/'` on Linux and Mac, `'\'` on Windows). Example: ```lua -import("stuff.lua") -- executes Plugins/name/stuff.lua -import("./stuff.lua") -- executes Plugins/name/stuff.lua -import("../stuff.lua") -- tries to load Plugins/stuff.lua and errors -import("luac.out") -- tried to load Plugins/name/luac.out and errors because it contains non-utf8 data +require("stuff") -- executes Plugins/name/stuff.lua or $(dirname $CURR_FILE)/stuff.lua +require("dir.name") -- executes Plugins/name/dir/name.lua or $(dirname $CURR_FILE)/dir/name.lua +require("binary") -- tried to load Plugins/name/binary.lua and errors because binary is not a text file ``` #### `print(Args...)` diff --git a/src/controllers/plugins/LuaAPI.cpp b/src/controllers/plugins/LuaAPI.cpp index e34e063b4..d8c1b676c 100644 --- a/src/controllers/plugins/LuaAPI.cpp +++ b/src/controllers/plugins/LuaAPI.cpp @@ -15,6 +15,7 @@ # include # include # include +# include namespace { using namespace chatterino; @@ -282,69 +283,87 @@ int g_load(lua_State *L) # endif } -int g_import(lua_State *L) +int loadfile(lua_State *L, const QString &str) { - auto countArgs = lua_gettop(L); - // Lua allows dofile() which loads from stdin, but this is very useless in our case - if (countArgs == 0) - { - lua_pushnil(L); - luaL_error(L, "it is not allowed to call import() without arguments"); - return 1; - } - auto *pl = getApp()->plugins->getPluginByStatePtr(L); - QString fname; - if (!lua::pop(L, &fname)) + if (pl == nullptr) { - lua_pushnil(L); - luaL_error(L, "chatterino g_import: expected a string for a filename"); - return 1; + return luaL_error(L, "loadfile: internal error: no plugin?"); } auto dir = QUrl(pl->loadDirectory().canonicalPath() + "/"); - auto file = dir.resolved(fname); - qCDebug(chatterinoLua) << "plugin" << pl->id << "is trying to load" << file - << "(its dir is" << dir << ")"; - if (!dir.isParentOf(file)) + if (!dir.isParentOf(str)) { - lua_pushnil(L); - luaL_error(L, "chatterino g_import: filename must be inside of the " - "plugin directory"); + // XXX: This intentionally hides the resolved path to not leak it + lua::push( + L, QString("requested module is outside of the plugin directory")); + return 1; + } + QFileInfo info(str); + if (!info.exists()) + { + lua::push(L, QString("no file '%1'").arg(str)); return 1; } - auto path = file.path(QUrl::FullyDecoded); - QFile qf(path); - qf.open(QIODevice::ReadOnly); - if (qf.size() > 10'000'000) + auto temp = str.toStdString(); + const auto *filename = temp.c_str(); + + auto res = luaL_loadfilex(L, filename, "t"); + // Yoinked from checkload lib/lua/src/loadlib.c + if (res == LUA_OK) { - lua_pushnil(L); - luaL_error(L, "chatterino g_import: size limit of 10MB exceeded, what " - "the hell are you doing"); + lua_pushstring(L, filename); + return 2; + } + + return luaL_error(L, "error loading module '%s' from file '%s':\n\t%s", + lua_tostring(L, 1), filename, lua_tostring(L, -1)); +} + +int searcherAbsolute(lua_State *L) +{ + auto name = QString::fromUtf8(luaL_checkstring(L, 1)); + name = name.replace('.', QDir::separator()); + + QString filename; + auto *pl = getApp()->plugins->getPluginByStatePtr(L); + if (pl == nullptr) + { + return luaL_error(L, "searcherAbsolute: internal error: no plugin?"); + } + + QFileInfo file(pl->loadDirectory().filePath(name + ".lua")); + return loadfile(L, file.canonicalFilePath()); +} + +int searcherRelative(lua_State *L) +{ + lua_Debug dbg; + lua_getstack(L, 1, &dbg); + lua_getinfo(L, "S", &dbg); + auto currentFile = QString::fromUtf8(dbg.source, dbg.srclen); + if (currentFile.startsWith("@")) + { + currentFile = currentFile.mid(1); + } + if (currentFile == "=[C]" || currentFile == "") + { + lua::push( + L, + QString( + "Unable to load relative to file:caller has no source file")); return 1; } - // validate utf-8 to block bytecode exploits - auto data = qf.readAll(); - auto *utf8 = QTextCodec::codecForName("UTF-8"); - QTextCodec::ConverterState state; - utf8->toUnicode(data.constData(), data.size(), &state); - if (state.invalidChars != 0) - { - lua_pushnil(L); - luaL_error(L, "invalid utf-8 in import() target (%s) is not allowed", - fname.toStdString().c_str()); - return 1; - } + auto parent = QFileInfo(currentFile).dir(); - // fetch dofile and call it - lua_getfield(L, LUA_REGISTRYINDEX, "real_dofile"); - // maybe data race here if symlink was swapped? - lua::push(L, path); - lua_call(L, 1, LUA_MULTRET); + auto name = QString::fromUtf8(luaL_checkstring(L, 1)); + name = name.replace('.', QDir::separator()); + QString filename = + parent.canonicalPath() + QDir::separator() + name + ".lua"; - return lua_gettop(L); + return loadfile(L, filename); } int g_print(lua_State *L) diff --git a/src/controllers/plugins/LuaAPI.hpp b/src/controllers/plugins/LuaAPI.hpp index 95a9bd192..155afc707 100644 --- a/src/controllers/plugins/LuaAPI.hpp +++ b/src/controllers/plugins/LuaAPI.hpp @@ -20,9 +20,12 @@ int c2_log(lua_State *L); // These ones are global int g_load(lua_State *L); int g_print(lua_State *L); -int g_import(lua_State *L); // NOLINTEND(readability-identifier-naming) +// This is for require() exposed as an element of package.searchers +int searcherAbsolute(lua_State *L); +int searcherRelative(lua_State *L); + // Exposed as c2.LogLevel // Represents "calls" to qCDebug, qCInfo ... enum class LogLevel { Debug, Info, Warning, Critical }; diff --git a/src/controllers/plugins/PluginController.cpp b/src/controllers/plugins/PluginController.cpp index f3829ac26..55def19ce 100644 --- a/src/controllers/plugins/PluginController.cpp +++ b/src/controllers/plugins/PluginController.cpp @@ -103,9 +103,10 @@ bool PluginController::tryLoadFromDir(const QDir &pluginDir) return true; } -void PluginController::openLibrariesFor(lua_State *L, - const PluginMeta & /*meta*/) +void PluginController::openLibrariesFor(lua_State *L, const PluginMeta &meta, + const QDir &pluginDir) { + lua::StackGuard guard(L); // Stuff to change, remove or hide behind a permission system: static const std::vector loadedlibs = { luaL_Reg{LUA_GNAME, luaopen_base}, @@ -123,6 +124,7 @@ void PluginController::openLibrariesFor(lua_State *L, luaL_Reg{LUA_STRLIBNAME, luaopen_string}, luaL_Reg{LUA_MATHLIBNAME, luaopen_math}, luaL_Reg{LUA_UTF8LIBNAME, luaopen_utf8}, + luaL_Reg{LUA_LOADLIBNAME, luaopen_package}, }; // Warning: Do not add debug library to this, it would make the security of // this a living nightmare due to stuff like registry access @@ -144,7 +146,7 @@ void PluginController::openLibrariesFor(lua_State *L, {nullptr, nullptr}, }; lua_pushglobaltable(L); - auto global = lua_gettop(L); + auto gtable = lua_gettop(L); // count of elements in C2LIB + LogLevel + EventType auto c2libIdx = lua::pushEmptyTable(L, 8); @@ -157,14 +159,11 @@ void PluginController::openLibrariesFor(lua_State *L, lua::pushEnumTable(L); lua_setfield(L, c2libIdx, "EventType"); - lua_setfield(L, global, "c2"); + lua_setfield(L, gtable, "c2"); // ban functions // Note: this might not be fully secure? some kind of metatable fuckery might come up? - lua_pushglobaltable(L); - auto gtable = lua_gettop(L); - // possibly randomize this name at runtime to prevent some attacks? # ifndef NDEBUG @@ -172,16 +171,10 @@ void PluginController::openLibrariesFor(lua_State *L, lua_setfield(L, LUA_REGISTRYINDEX, "real_load"); # endif - lua_getfield(L, gtable, "dofile"); - lua_setfield(L, LUA_REGISTRYINDEX, "real_dofile"); - // NOLINTNEXTLINE(*-avoid-c-arrays) static const luaL_Reg replacementFuncs[] = { {"load", lua::api::g_load}, {"print", lua::api::g_print}, - - // This function replaces both `dofile` and `require`, see docs/wip-plugins.md for more info - {"import", lua::api::g_import}, {nullptr, nullptr}, }; luaL_setfuncs(L, replacementFuncs, 0); @@ -192,7 +185,43 @@ void PluginController::openLibrariesFor(lua_State *L, lua_pushnil(L); lua_setfield(L, gtable, "dofile"); - lua_pop(L, 1); + // set up package lib + lua_getfield(L, gtable, "package"); + + auto package = lua_gettop(L); + lua_pushstring(L, ""); + lua_setfield(L, package, "cpath"); + + // we don't use path + lua_pushstring(L, ""); + lua_setfield(L, package, "path"); + + { + lua_getfield(L, gtable, "table"); + auto table = lua_gettop(L); + lua_getfield(L, -1, "remove"); + lua_remove(L, table); + } + auto remove = lua_gettop(L); + + // remove searcher_Croot, searcher_C and searcher_Lua leaving only searcher_preload + for (int i = 0; i < 3; i++) + { + lua_pushvalue(L, remove); + lua_getfield(L, package, "searchers"); + lua_pcall(L, 1, 0, 0); + } + lua_pop(L, 1); // get rid of remove + + lua_getfield(L, package, "searchers"); + lua_pushcclosure(L, lua::api::searcherRelative, 0); + lua_seti(L, -2, 2); + + lua::push(L, QString(pluginDir.absolutePath())); + lua_pushcclosure(L, lua::api::searcherAbsolute, 1); + lua_seti(L, -2, 3); + + lua_pop(L, 3); // remove gtable, package, package.searchers } void PluginController::load(const QFileInfo &index, const QDir &pluginDir, @@ -211,7 +240,7 @@ void PluginController::load(const QFileInfo &index, const QDir &pluginDir, << " because safe mode is enabled."; return; } - PluginController::openLibrariesFor(l, meta); + PluginController::openLibrariesFor(l, meta, pluginDir); if (!PluginController::isPluginEnabled(pluginName) || !getSettings()->pluginsEnabled) diff --git a/src/controllers/plugins/PluginController.hpp b/src/controllers/plugins/PluginController.hpp index bc3eca738..8dc698b40 100644 --- a/src/controllers/plugins/PluginController.hpp +++ b/src/controllers/plugins/PluginController.hpp @@ -63,7 +63,8 @@ private: const PluginMeta &meta); // This function adds lua standard libraries into the state - static void openLibrariesFor(lua_State *L, const PluginMeta & /*meta*/); + static void openLibrariesFor(lua_State *L, const PluginMeta & /*meta*/, + const QDir &pluginDir); static void loadChatterinoLib(lua_State *l); bool tryLoadFromDir(const QDir &pluginDir); std::map> plugins_;