From 8c99f2232bdb52459ccf2a5b751cbe3f7797abc3 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Fri, 17 Dec 2021 18:31:29 +0100 Subject: [PATCH] Don't let HTTP API pass through untrusted function This has been a problem since the first day, oops. --- builtin/game/misc.lua | 5 +++-- src/script/common/c_internal.h | 2 ++ src/script/lua_api/l_http.cpp | 23 +++++++++++++++++++---- src/script/lua_api/l_http.h | 3 +++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/builtin/game/misc.lua b/builtin/game/misc.lua index ef826eda7..e86efc50c 100644 --- a/builtin/game/misc.lua +++ b/builtin/game/misc.lua @@ -250,7 +250,7 @@ end -- HTTP callback interface -function core.http_add_fetch(httpenv) +core.set_http_api_lua(function(httpenv) httpenv.fetch = function(req, callback) local handle = httpenv.fetch_async(req) @@ -266,7 +266,8 @@ function core.http_add_fetch(httpenv) end return httpenv -end +end) +core.set_http_api_lua = nil function core.close_formspec(player_name, formname) diff --git a/src/script/common/c_internal.h b/src/script/common/c_internal.h index ab2d7b975..94cfd61fb 100644 --- a/src/script/common/c_internal.h +++ b/src/script/common/c_internal.h @@ -54,6 +54,8 @@ extern "C" { #define CUSTOM_RIDX_GLOBALS_BACKUP (CUSTOM_RIDX_BASE + 1) #define CUSTOM_RIDX_CURRENT_MOD_NAME (CUSTOM_RIDX_BASE + 2) #define CUSTOM_RIDX_BACKTRACE (CUSTOM_RIDX_BASE + 3) +#define CUSTOM_RIDX_HTTP_API_LUA (CUSTOM_RIDX_BASE + 4) + // Determine if CUSTOM_RIDX_SCRIPTAPI will hold a light or full userdata #if defined(__aarch64__) && USE_LUAJIT diff --git a/src/script/lua_api/l_http.cpp b/src/script/lua_api/l_http.cpp index 751ec9837..b385b698c 100644 --- a/src/script/lua_api/l_http.cpp +++ b/src/script/lua_api/l_http.cpp @@ -163,6 +163,20 @@ int ModApiHttp::l_http_fetch_async_get(lua_State *L) return 1; } +int ModApiHttp::l_set_http_api_lua(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + + // This is called by builtin to give us a function that will later + // populate the http_api table with additional method(s). + // We need this because access to the HTTP api is security-relevant and + // any mod could just mess with a global variable. + luaL_checktype(L, 1, LUA_TFUNCTION); + lua_rawseti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_HTTP_API_LUA); + + return 0; +} + int ModApiHttp::l_request_http_api(lua_State *L) { NO_MAP_LOCK_REQUIRED; @@ -205,16 +219,16 @@ int ModApiHttp::l_request_http_api(lua_State *L) return 1; } - lua_getglobal(L, "core"); - lua_getfield(L, -1, "http_add_fetch"); + lua_rawgeti(L, LUA_REGISTRYINDEX, CUSTOM_RIDX_HTTP_API_LUA); + assert(lua_isfunction(L, -1)); lua_newtable(L); HTTP_API(fetch_async); HTTP_API(fetch_async_get); // Stack now looks like this: - // - // Now call core.http_add_fetch to append .fetch(request, callback) to table + //
+ // Now call it to append .fetch(request, callback) to table lua_call(L, 1, 1); return 1; @@ -247,6 +261,7 @@ void ModApiHttp::Initialize(lua_State *L, int top) API_FCT(get_http_api); } else { API_FCT(request_http_api); + API_FCT(set_http_api_lua); } #endif diff --git a/src/script/lua_api/l_http.h b/src/script/lua_api/l_http.h index c3a2a5276..17fa283ba 100644 --- a/src/script/lua_api/l_http.h +++ b/src/script/lua_api/l_http.h @@ -41,6 +41,9 @@ private: // http_fetch_async_get(handle) static int l_http_fetch_async_get(lua_State *L); + // set_http_api_lua() [internal] + static int l_set_http_api_lua(lua_State *L); + // request_http_api() static int l_request_http_api(lua_State *L);