giomodule: Ignore GIO_MODULE_DIR when running as setuid

Even if the modules in the given directory never get chosen to be used,
loading arbitrary code from a user-provided directory is not safe when
running as setuid, as the process’ environment comes from an untrusted
source.

Also ignore `GIO_EXTRA_MODULES`.

Spotted by Simon McVittie.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fixes: #2168
This commit is contained in:
Philip Withnall 2020-12-04 23:33:43 +00:00
parent 95729db0e1
commit ba414ee100
2 changed files with 25 additions and 3 deletions

View File

@ -436,6 +436,9 @@ Gvfs is also heavily distributed and relies on a session bus to be present.
modules from this alternate directory instead of the directory modules from this alternate directory instead of the directory
built into GIO. This is useful when running tests, for example. built into GIO. This is useful when running tests, for example.
</para> </para>
<para>
This environment variable is ignored when running in a setuid program.
</para>
</formalpara> </formalpara>
<formalpara> <formalpara>
@ -446,6 +449,9 @@ Gvfs is also heavily distributed and relies on a session bus to be present.
paths separated by a colon, GIO will attempt to load paths separated by a colon, GIO will attempt to load
additional modules from within the path. additional modules from within the path.
</para> </para>
<para>
This environment variable is ignored when running in a setuid program.
</para>
</formalpara> </formalpara>
<formalpara> <formalpara>

View File

@ -30,6 +30,7 @@
#include "giomodule.h" #include "giomodule.h"
#include "giomodule-priv.h" #include "giomodule-priv.h"
#include "glib-private.h"
#include "glocalfilemonitor.h" #include "glocalfilemonitor.h"
#include "gnativevolumemonitor.h" #include "gnativevolumemonitor.h"
#include "gproxyresolver.h" #include "gproxyresolver.h"
@ -812,6 +813,9 @@ _g_io_module_get_default_type (const gchar *extension_point,
return G_TYPE_INVALID; return G_TYPE_INVALID;
} }
/* Its OK to query the environment here, even when running as setuid, because
* it only allows a choice between existing already-loaded modules. No new
* code is loaded based on the environment variable value. */
use_this = envvar ? g_getenv (envvar) : NULL; use_this = envvar ? g_getenv (envvar) : NULL;
if (g_strcmp0 (use_this, "help") == 0) if (g_strcmp0 (use_this, "help") == 0)
{ {
@ -961,6 +965,9 @@ _g_io_module_get_default (const gchar *extension_point,
return NULL; return NULL;
} }
/* Its OK to query the environment here, even when running as setuid, because
* it only allows a choice between existing already-loaded modules. No new
* code is loaded based on the environment variable value. */
use_this = envvar ? g_getenv (envvar) : NULL; use_this = envvar ? g_getenv (envvar) : NULL;
if (g_strcmp0 (use_this, "help") == 0) if (g_strcmp0 (use_this, "help") == 0)
{ {
@ -1159,8 +1166,16 @@ static gchar *
get_gio_module_dir (void) get_gio_module_dir (void)
{ {
gchar *module_dir; gchar *module_dir;
gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
module_dir = g_strdup (g_getenv ("GIO_MODULE_DIR")); /* If running as setuid, loading modules from an arbitrary directory
* controlled by the unprivileged user who is running the program could allow
* for execution of arbitrary code (in constructors in modules).
* Dont allow it.
*
* If a setuid program somehow needs to load additional GIO modules, it should
* explicitly call g_io_modules_scan_all_in_directory(). */
module_dir = !is_setuid ? g_strdup (g_getenv ("GIO_MODULE_DIR")) : NULL;
if (module_dir == NULL) if (module_dir == NULL)
{ {
#ifdef G_OS_WIN32 #ifdef G_OS_WIN32
@ -1192,13 +1207,14 @@ _g_io_modules_ensure_loaded (void)
if (!loaded_dirs) if (!loaded_dirs)
{ {
gboolean is_setuid = GLIB_PRIVATE_CALL (g_check_setuid) ();
gchar *module_dir; gchar *module_dir;
loaded_dirs = TRUE; loaded_dirs = TRUE;
scope = g_io_module_scope_new (G_IO_MODULE_SCOPE_BLOCK_DUPLICATES); scope = g_io_module_scope_new (G_IO_MODULE_SCOPE_BLOCK_DUPLICATES);
/* First load any overrides, extras */ /* First load any overrides, extras (but not if running as setuid!) */
module_path = g_getenv ("GIO_EXTRA_MODULES"); module_path = !is_setuid ? g_getenv ("GIO_EXTRA_MODULES") : NULL;
if (module_path) if (module_path)
{ {
gchar **paths; gchar **paths;