portal: Let --env= and --env-fd= take precedence over extra-args

Previously, if you launched a subsandbox while specifying environment
variable overrides, any environment variable overrides that existed
in the parent Flatpak app would take precedence:

    host$ flatpak run --env=FOO=1 --command=bash example.app
    [📦 example.app ~]$ env | grep FOO
    FOO=1
    [📦 example.app ~]$ flatpak-spawn --env=FOO=x sh -c 'env | grep FOO'
    FOO=1

This does not seem like least-astonishment, and in particular will
cause problems if the app wants to override LD_LIBRARY_PATH in the
subsandbox. Change the precedence so that the environment variables
set by flatpak-spawn will "win":

    host$ flatpak run --env=FOO1=1 --env=FOO2=2 --command=bash example.app
    [📦 example.app ~]$ env | grep FOO
    FOO1=1
    FOO2=2
    [📦 example.app ~]$ flatpak-spawn --env=FOO1=x sh -c 'env | grep FOO'
    FOO1=x
    FOO2=2

This follows up from GHSA-4ppf-fxf6-vxg2 to fix an issue that I noticed
while resolving that vulnerability, but is not required for fixing the
vulnerability.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie
2021-01-11 16:40:43 +00:00
committed by Alexander Larsson
parent 4ac1106690
commit e2c4ded323

View File

@@ -1003,42 +1003,6 @@ handle_spawn (PortalFlatpak *object,
else
env = g_get_environ ();
/* Let the environment variables given by the caller override the ones
* from extra_args. Don't add them to @env, because they are controlled
* by our caller, which might be trying to use them to inject code into
* flatpak(1); add them to the environment block instead.
*
* We don't use --env= here, so that if the values are something that
* should not be exposed to other uids, they can remain confidential. */
n_envs = g_variant_n_children (arg_envs);
for (i = 0; i < n_envs; i++)
{
const char *var = NULL;
const char *val = NULL;
g_variant_get_child (arg_envs, i, "{&s&s}", &var, &val);
if (var[0] == '\0')
{
g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR,
G_DBUS_ERROR_INVALID_ARGS,
"Environment variable cannot have empty name");
return G_DBUS_METHOD_INVOCATION_HANDLED;
}
if (strchr (var, '=') != NULL)
{
g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR,
G_DBUS_ERROR_INVALID_ARGS,
"Environment variable name cannot contain '='");
return G_DBUS_METHOD_INVOCATION_HANDLED;
}
g_string_append (env_string, var);
g_string_append_c (env_string, '=');
g_string_append (env_string, val);
g_string_append_c (env_string, '\0');
}
g_ptr_array_add (flatpak_argv, g_strdup ("flatpak"));
g_ptr_array_add (flatpak_argv, g_strdup ("run"));
@@ -1108,6 +1072,42 @@ handle_spawn (PortalFlatpak *object,
}
}
/* Let the environment variables given by the caller override the ones
* from extra_args. Don't add them to @env, because they are controlled
* by our caller, which might be trying to use them to inject code into
* flatpak(1); add them to the environment block instead.
*
* We don't use --env= here, so that if the values are something that
* should not be exposed to other uids, they can remain confidential. */
n_envs = g_variant_n_children (arg_envs);
for (i = 0; i < n_envs; i++)
{
const char *var = NULL;
const char *val = NULL;
g_variant_get_child (arg_envs, i, "{&s&s}", &var, &val);
if (var[0] == '\0')
{
g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR,
G_DBUS_ERROR_INVALID_ARGS,
"Environment variable cannot have empty name");
return G_DBUS_METHOD_INVOCATION_HANDLED;
}
if (strchr (var, '=') != NULL)
{
g_dbus_method_invocation_return_error (invocation, G_DBUS_ERROR,
G_DBUS_ERROR_INVALID_ARGS,
"Environment variable name cannot contain '='");
return G_DBUS_METHOD_INVOCATION_HANDLED;
}
g_string_append (env_string, var);
g_string_append_c (env_string, '=');
g_string_append (env_string, val);
g_string_append_c (env_string, '\0');
}
if (env_string->len > 0)
{
g_auto(GLnxTmpfile) env_tmpf = { 0, };