From 9b25bed85a693c0979cf92925ca080310fe566b1 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 12 Jan 2021 12:21:31 +0000
Subject: [PATCH 1/5] run: Convert all environment variables into bwrap
arguments
This avoids some of them being filtered out by a setuid bwrap. It also
means that if they came from an untrusted source, they cannot be used
to inject arbitrary code into a non-setuid bwrap via mechanisms like
LD_PRELOAD.
Because they get bundled into a memfd or temporary file, they do not
actually appear in argv, ensuring that they remain inaccessible to
processes running under a different uid (which is important if their
values are tokens or other secrets).
[Backported to 1.2.x for Debian 10 security update.]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
common/flatpak-bwrap-private.h | 3 +++
common/flatpak-bwrap.c | 43 ++++++++++++++++++++++++++++++++++
common/flatpak-run.c | 24 ++++++++++++-------
3 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/common/flatpak-bwrap-private.h b/common/flatpak-bwrap-private.h
index 2a633d30..b6f3df9f 100644
--- a/common/flatpak-bwrap-private.h
+++ b/common/flatpak-bwrap-private.h
@@ -43,6 +43,8 @@ void flatpak_bwrap_unset_env (FlatpakBwrap *bwrap,
const char *variable);
void flatpak_bwrap_add_arg (FlatpakBwrap *bwrap,
const char *arg);
+void flatpak_bwrap_take_arg (FlatpakBwrap *bwrap,
+ char *arg);
void flatpak_bwrap_add_noinherit_fd (FlatpakBwrap *bwrap,
int fd);
void flatpak_bwrap_add_fd (FlatpakBwrap *bwrap,
@@ -73,6 +75,7 @@ void flatpak_bwrap_add_bind_arg (FlatpakBwrap *bwrap,
const char *type,
const char *src,
const char *dest);
+void flatpak_bwrap_envp_to_args (FlatpakBwrap *bwrap);
gboolean flatpak_bwrap_bundle_args (FlatpakBwrap *bwrap,
int start,
int end,
diff --git a/common/flatpak-bwrap.c b/common/flatpak-bwrap.c
index 826fd621..0194db84 100644
--- a/common/flatpak-bwrap.c
+++ b/common/flatpak-bwrap.c
@@ -108,6 +108,18 @@ flatpak_bwrap_add_arg (FlatpakBwrap *bwrap, const char *arg)
g_ptr_array_add (bwrap->argv, g_strdup (arg));
}
+/*
+ * flatpak_bwrap_take_arg:
+ * @arg: (transfer full): Take ownership of this argument
+ *
+ * Add @arg to @bwrap's argv, taking ownership of the pointer.
+ */
+void
+flatpak_bwrap_take_arg (FlatpakBwrap *bwrap, char *arg)
+{
+ g_ptr_array_add (bwrap->argv, arg);
+}
+
void
flatpak_bwrap_finish (FlatpakBwrap *bwrap)
{
@@ -273,6 +285,37 @@ flatpak_bwrap_add_bind_arg (FlatpakBwrap *bwrap,
}
}
+/*
+ * Convert bwrap->envp into a series of --setenv arguments for bwrap(1),
+ * assumed to be applied to an empty environment. Reset envp to be an
+ * empty environment.
+ */
+void
+flatpak_bwrap_envp_to_args (FlatpakBwrap *bwrap)
+{
+ gsize i;
+
+ for (i = 0; bwrap->envp[i] != NULL; i++)
+ {
+ char *key_val = bwrap->envp[i];
+ char *eq = strchr (key_val, '=');
+
+ if (eq)
+ {
+ flatpak_bwrap_add_arg (bwrap, "--setenv");
+ flatpak_bwrap_take_arg (bwrap, g_strndup (key_val, eq - key_val));
+ flatpak_bwrap_add_arg (bwrap, eq + 1);
+ }
+ else
+ {
+ g_warn_if_reached ();
+ }
+ }
+
+ g_strfreev (g_steal_pointer (&bwrap->envp));
+ bwrap->envp = g_strdupv (flatpak_bwrap_empty_env);
+}
+
gboolean
flatpak_bwrap_bundle_args (FlatpakBwrap *bwrap,
int start,
diff --git a/common/flatpak-run.c b/common/flatpak-run.c
index 08bfe54a..ea5571bd 100644
--- a/common/flatpak-run.c
+++ b/common/flatpak-run.c
@@ -1120,15 +1120,6 @@ flatpak_run_add_environment_args (FlatpakBwrap *bwrap,
flatpak_run_add_system_dbus_args (bwrap, proxy_arg_bwrap, context, flags);
flatpak_run_add_a11y_dbus_args (bwrap, proxy_arg_bwrap, context, flags);
- if (g_environ_getenv (bwrap->envp, "LD_LIBRARY_PATH") != NULL)
- {
- /* LD_LIBRARY_PATH is overridden for setuid helper, so pass it as cmdline arg */
- flatpak_bwrap_add_args (bwrap,
- "--setenv", "LD_LIBRARY_PATH", g_environ_getenv (bwrap->envp, "LD_LIBRARY_PATH"),
- NULL);
- flatpak_bwrap_unset_env (bwrap, "LD_LIBRARY_PATH");
- }
-
/* Must run this before spawning the dbus proxy, to ensure it
ends up in the app cgroup */
if (!flatpak_run_in_transient_unit (app_id, &my_error))
@@ -3139,6 +3130,8 @@ flatpak_run_app (const char *app_ref,
command = default_command;
}
+ flatpak_bwrap_envp_to_args (bwrap);
+
if (!flatpak_bwrap_bundle_args (bwrap, 1, -1, FALSE, error))
return FALSE;
@@ -3161,6 +3154,12 @@ flatpak_run_app (const char *app_ref,
char pid_str[64];
g_autofree char *pid_path = NULL;
+ /* flatpak_bwrap_envp_to_args() moved the environment variables to
+ * be set into --setenv instructions in argv, so the environment
+ * in which the bwrap command runs must be empty. */
+ g_assert (bwrap->envp != NULL);
+ g_assert (bwrap->envp[0] == NULL);
+
if (!g_spawn_async (NULL,
(char **) bwrap->argv->pdata,
bwrap->envp,
@@ -3185,6 +3184,13 @@ flatpak_run_app (const char *app_ref,
/* Ensure we unset O_CLOEXEC */
flatpak_bwrap_child_setup_cb (bwrap->fds);
+
+ /* flatpak_bwrap_envp_to_args() moved the environment variables to
+ * be set into --setenv instructions in argv, so the environment
+ * in which the bwrap command runs must be empty. */
+ g_assert (bwrap->envp != NULL);
+ g_assert (bwrap->envp[0] == NULL);
+
if (execvpe (flatpak_get_bwrap (), (char **) bwrap->argv->pdata, bwrap->envp) == -1)
{
g_set_error_literal (error, G_IO_ERROR, g_io_error_from_errno (errno),
--
2.29.2
From 260ae2c2ed5066762ba108f95210a9d9a66b99d4 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Mon, 11 Jan 2021 14:51:36 +0000
Subject: [PATCH 2/5] common: Move flatpak_buffer_to_sealed_memfd_or_tmpfile to
its own file
We'll need this to use it in flatpak-portal without pulling the rest of
the common/ directory.
[Part of a 1.2.x backport of GHSA-4ppf-fxf6-vxg2 for Debian 10.]
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
common/Makefile.am.inc | 2 +
common/flatpak-utils-memfd-private.h | 32 ++++++++++
common/flatpak-utils-memfd.c | 90 ++++++++++++++++++++++++++++
common/flatpak-utils-private.h | 1 +
common/flatpak-utils.c | 50 ----------------
5 files changed, 125 insertions(+), 50 deletions(-)
create mode 100644 common/flatpak-utils-memfd-private.h
create mode 100644 common/flatpak-utils-memfd.c
diff --git a/common/Makefile.am.inc b/common/Makefile.am.inc
index f3966903..794bd4e3 100644
--- a/common/Makefile.am.inc
+++ b/common/Makefile.am.inc
@@ -96,6 +96,8 @@ libflatpak_common_la_SOURCES = \
common/flatpak-utils.c \
common/flatpak-utils-http.c \
common/flatpak-utils-http-private.h \
+ common/flatpak-utils-memfd.c \
+ common/flatpak-utils-memfd-private.h \
common/flatpak-utils-private.h \
common/flatpak-chain-input-stream.c \
common/flatpak-chain-input-stream-private.h \
diff --git a/common/flatpak-utils-memfd-private.h b/common/flatpak-utils-memfd-private.h
new file mode 100644
index 00000000..c0e985f0
--- /dev/null
+++ b/common/flatpak-utils-memfd-private.h
@@ -0,0 +1,32 @@
+/*
+ * Copyright © 2014 Red Hat, Inc
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ * Alexander Larsson <alexl@redhat.com>
+ */
+
+#ifndef __FLATPAK_UTILS_MEMFD_H__
+#define __FLATPAK_UTILS_MEMFD_H__
+
+#include "libglnx/libglnx.h"
+
+gboolean flatpak_buffer_to_sealed_memfd_or_tmpfile (GLnxTmpfile *tmpf,
+ const char *name,
+ const char *str,
+ size_t len,
+ GError **error);
+
+#endif /* __FLATPAK_UTILS_MEMFD_H__ */
diff --git a/common/flatpak-utils-memfd.c b/common/flatpak-utils-memfd.c
new file mode 100644
index 00000000..9a0730f8
--- /dev/null
+++ b/common/flatpak-utils-memfd.c
@@ -0,0 +1,90 @@
+/*
+ * Copyright © 2014 Red Hat, Inc
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ * Alexander Larsson <alexl@redhat.com>
+ */
+
+#include "config.h"
+
+#include "flatpak-utils-memfd-private.h"
+
+#include "valgrind-private.h"
+
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/file.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/utsname.h>
+#include <sys/ioctl.h>
+#include <termios.h>
+
+/* If memfd_create() is available, generate a sealed memfd with contents of
+ * @str. Otherwise use an O_TMPFILE @tmpf in anonymous mode, write @str to
+ * @tmpf, and lseek() back to the start. See also similar uses in e.g.
+ * rpm-ostree for running dracut.
+ */
+gboolean
+flatpak_buffer_to_sealed_memfd_or_tmpfile (GLnxTmpfile *tmpf,
+ const char *name,
+ const char *str,
+ size_t len,
+ GError **error)
+{
+ if (len == -1)
+ len = strlen (str);
+ glnx_autofd int memfd = memfd_create (name, MFD_CLOEXEC | MFD_ALLOW_SEALING);
+ int fd; /* Unowned */
+ if (memfd != -1)
+ {
+ fd = memfd;
+ }
+ else
+ {
+ /* We use an anonymous fd (i.e. O_EXCL) since we don't want
+ * the target container to potentially be able to re-link it.
+ */
+ if (!G_IN_SET (errno, ENOSYS, EOPNOTSUPP))
+ return glnx_throw_errno_prefix (error, "memfd_create");
+ if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, tmpf, error))
+ return FALSE;
+ fd = tmpf->fd;
+ }
+ if (ftruncate (fd, len) < 0)
+ return glnx_throw_errno_prefix (error, "ftruncate");
+ if (glnx_loop_write (fd, str, len) < 0)
+ return glnx_throw_errno_prefix (error, "write");
+ if (lseek (fd, 0, SEEK_SET) < 0)
+ return glnx_throw_errno_prefix (error, "lseek");
+ if (memfd != -1)
+ {
+ /* Valgrind doesn't currently handle G_ADD_SEALS, so lets not seal when debugging... */
+ if ((!RUNNING_ON_VALGRIND) &&
+ fcntl (memfd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE | F_SEAL_SEAL) < 0)
+ return glnx_throw_errno_prefix (error, "fcntl(F_ADD_SEALS)");
+ /* The other values can stay default */
+ tmpf->fd = glnx_steal_fd (&memfd);
+ tmpf->initialized = TRUE;
+ }
+ return TRUE;
+}
diff --git a/common/flatpak-utils-private.h b/common/flatpak-utils-private.h
index 8ed25f5a..3e2bcef7 100644
--- a/common/flatpak-utils-private.h
+++ b/common/flatpak-utils-private.h
@@ -32,6 +32,7 @@
#include "flatpak-context-private.h"
#include "flatpak-error.h"
#include "flatpak-utils-http-private.h"
+#include "flatpak-utils-memfd-private.h"
#include <ostree.h>
#include <json-glib/json-glib.h>
diff --git a/common/flatpak-utils.c b/common/flatpak-utils.c
index 0c8eaa9d..698f943d 100644
--- a/common/flatpak-utils.c
+++ b/common/flatpak-utils.c
@@ -2280,56 +2280,6 @@ flatpak_file_rename (GFile *from,
return TRUE;
}
-/* If memfd_create() is available, generate a sealed memfd with contents of
- * @str. Otherwise use an O_TMPFILE @tmpf in anonymous mode, write @str to
- * @tmpf, and lseek() back to the start. See also similar uses in e.g.
- * rpm-ostree for running dracut.
- */
-gboolean
-flatpak_buffer_to_sealed_memfd_or_tmpfile (GLnxTmpfile *tmpf,
- const char *name,
- const char *str,
- size_t len,
- GError **error)
-{
- if (len == -1)
- len = strlen (str);
- glnx_autofd int memfd = memfd_create (name, MFD_CLOEXEC | MFD_ALLOW_SEALING);
- int fd; /* Unowned */
- if (memfd != -1)
- {
- fd = memfd;
- }
- else
- {
- /* We use an anonymous fd (i.e. O_EXCL) since we don't want
- * the target container to potentially be able to re-link it.
- */
- if (!G_IN_SET (errno, ENOSYS, EOPNOTSUPP))
- return glnx_throw_errno_prefix (error, "memfd_create");
- if (!glnx_open_anonymous_tmpfile (O_RDWR | O_CLOEXEC, tmpf, error))
- return FALSE;
- fd = tmpf->fd;
- }
- if (ftruncate (fd, len) < 0)
- return glnx_throw_errno_prefix (error, "ftruncate");
- if (glnx_loop_write (fd, str, len) < 0)
- return glnx_throw_errno_prefix (error, "write");
- if (lseek (fd, 0, SEEK_SET) < 0)
- return glnx_throw_errno_prefix (error, "lseek");
- if (memfd != -1)
- {
- /* Valgrind doesn't currently handle G_ADD_SEALS, so lets not seal when debugging... */
- if ((!RUNNING_ON_VALGRIND) &&
- fcntl (memfd, F_ADD_SEALS, F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE | F_SEAL_SEAL) < 0)
- return glnx_throw_errno_prefix (error, "fcntl(F_ADD_SEALS)");
- /* The other values can stay default */
- tmpf->fd = glnx_steal_fd (&memfd);
- tmpf->initialized = TRUE;
- }
- return TRUE;
-}
-
gboolean
flatpak_open_in_tmpdir_at (int tmpdir_fd,
int mode,
--
2.29.2
From ae30b3e44ed8cf16534afabef851f07005c83be1 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Sun, 10 Jan 2021 16:18:58 +0000
Subject: [PATCH 3/5] context: Add --env-fd option
This allows environment variables to be added to the context without
making their values visible to processes running under a different uid,
which might be significant if the variable's value is a token or some
other secret value.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
common/flatpak-context.c | 60 ++++++++++++++++++++++++++++++++++++
doc/flatpak-build-finish.xml | 18 +++++++++++
doc/flatpak-build.xml | 18 +++++++++++
doc/flatpak-override.xml | 18 +++++++++++
doc/flatpak-run.xml | 18 +++++++++++
5 files changed, 132 insertions(+)
diff --git a/common/flatpak-context.c b/common/flatpak-context.c
index 7289368d..b597bd1e 100644
--- a/common/flatpak-context.c
+++ b/common/flatpak-context.c
@@ -1039,6 +1039,65 @@ option_env_cb (const gchar *option_name,
return TRUE;
}
+static gboolean
+option_env_fd_cb (const gchar *option_name,
+ const gchar *value,
+ gpointer data,
+ GError **error)
+{
+ FlatpakContext *context = data;
+ g_autoptr(GBytes) env_block = NULL;
+ gsize remaining;
+ const char *p;
+ guint64 fd;
+ gchar *endptr;
+
+ fd = g_ascii_strtoull (value, &endptr, 10);
+
+ if (endptr == NULL || *endptr != '\0' || fd > G_MAXINT)
+ return glnx_throw (error, "Not a valid file descriptor: %s", value);
+
+ env_block = glnx_fd_readall_bytes ((int) fd, NULL, error);
+
+ if (env_block == NULL)
+ return FALSE;
+
+ p = g_bytes_get_data (env_block, &remaining);
+
+ /* env_block might not be \0-terminated */
+ while (remaining > 0)
+ {
+ size_t len = strnlen (p, remaining);
+ const char *equals;
+
+ g_assert (len <= remaining);
+
+ equals = memchr (p, '=', len);
+
+ if (equals == NULL || equals == p)
+ return glnx_throw (error,
+ "Environment variable must be given in the form VARIABLE=VALUE, not %.*s", (int) len, p);
+
+ flatpak_context_set_env_var (context,
+ g_strndup (p, equals - p),
+ g_strndup (equals + 1, len - (equals - p) - 1));
+ p += len;
+ remaining -= len;
+
+ if (remaining > 0)
+ {
+ g_assert (*p == '\0');
+ p += 1;
+ remaining -= 1;
+ }
+ }
+
+ if (fd >= 3)
+ close (fd);
+
+ return TRUE;
+}
+
static gboolean
option_own_name_cb (const gchar *option_name,
const gchar *value,
@@ -1206,6 +1265,7 @@ static GOptionEntry context_options[] = {
{ "filesystem", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_filesystem_cb, N_("Expose filesystem to app (:ro for read-only)"), N_("FILESYSTEM[:ro]") },
{ "nofilesystem", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_nofilesystem_cb, N_("Don't expose filesystem to app"), N_("FILESYSTEM") },
{ "env", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_env_cb, N_("Set environment variable"), N_("VAR=VALUE") },
+ { "env-fd", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_env_fd_cb, N_("Read environment variables in env -0 format from FD"), N_("FD") },
{ "own-name", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_own_name_cb, N_("Allow app to own name on the session bus"), N_("DBUS_NAME") },
{ "talk-name", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_talk_name_cb, N_("Allow app to talk to name on the session bus"), N_("DBUS_NAME") },
{ "system-own-name", 0, G_OPTION_FLAG_IN_MAIN, G_OPTION_ARG_CALLBACK, &option_system_own_name_cb, N_("Allow app to own name on the system bus"), N_("DBUS_NAME") },
diff --git a/doc/flatpak-build-finish.xml b/doc/flatpak-build-finish.xml
index efe39521..d3de7d93 100644
--- a/doc/flatpak-build-finish.xml
+++ b/doc/flatpak-build-finish.xml
@@ -276,6 +276,24 @@ key=v1;v2;
</para></listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+ <listitem><para>
+ Read environment variables from the file descriptor
+ <replaceable>FD</replaceable>, and set them as if
+ via <option>--env</option>. This can be used to avoid
+ environment variables and their values becoming visible
+ to other users.
+ </para><para>
+ Each environment variable is in the form
+ <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+ followed by a zero byte. This is the same format used by
+ <literal>env -0</literal> and
+ <filename>/proc/*/environ</filename>.
+ </para></listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--own-name=NAME</option></term>
diff --git a/doc/flatpak-build.xml b/doc/flatpak-build.xml
index 539314e1..488eef8a 100644
--- a/doc/flatpak-build.xml
+++ b/doc/flatpak-build.xml
@@ -283,6 +283,24 @@ key=v1;v2;
</para></listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+ <listitem><para>
+ Read environment variables from the file descriptor
+ <replaceable>FD</replaceable>, and set them as if
+ via <option>--env</option>. This can be used to avoid
+ environment variables and their values becoming visible
+ to other users.
+ </para><para>
+ Each environment variable is in the form
+ <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+ followed by a zero byte. This is the same format used by
+ <literal>env -0</literal> and
+ <filename>/proc/*/environ</filename>.
+ </para></listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--own-name=NAME</option></term>
diff --git a/doc/flatpak-override.xml b/doc/flatpak-override.xml
index c91875cc..60fa58c5 100644
--- a/doc/flatpak-override.xml
+++ b/doc/flatpak-override.xml
@@ -257,6 +257,24 @@ key=v1;v2;
</para></listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+ <listitem><para>
+ Read environment variables from the file descriptor
+ <replaceable>FD</replaceable>, and set them as if
+ via <option>--env</option>. This can be used to avoid
+ environment variables and their values becoming visible
+ to other users.
+ </para><para>
+ Each environment variable is in the form
+ <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+ followed by a zero byte. This is the same format used by
+ <literal>env -0</literal> and
+ <filename>/proc/*/environ</filename>.
+ </para></listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--own-name=NAME</option></term>
diff --git a/doc/flatpak-run.xml b/doc/flatpak-run.xml
index 23503e11..8e1a3fd5 100644
--- a/doc/flatpak-run.xml
+++ b/doc/flatpak-run.xml
@@ -360,6 +360,24 @@ key=v1;v2;
</para></listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--env-fd=<replaceable>FD</replaceable></option></term>
+
+ <listitem><para>
+ Read environment variables from the file descriptor
+ <replaceable>FD</replaceable>, and set them as if
+ via <option>--env</option>. This can be used to avoid
+ environment variables and their values becoming visible
+ to other users.
+ </para><para>
+ Each environment variable is in the form
+ <replaceable>VAR</replaceable>=<replaceable>VALUE</replaceable>
+ followed by a zero byte. This is the same format used by
+ <literal>env -0</literal> and
+ <filename>/proc/*/environ</filename>.
+ </para></listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--own-name=NAME</option></term>
--
2.29.2
From b781c377efb1c1782ac38019eba2ef17258ddaeb Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Tue, 12 Jan 2021 12:25:59 +0000
Subject: [PATCH 4/5] portal: Convert --env in extra-args into --env-fd
This hides overridden variables from the command-line, which means
processes running under other uids can't see them in /proc/*/cmdline,
which might be important if they contain secrets.
[Backported to 1.2.x for Debian 10 security update]
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
portal/Makefile.am.inc | 4 ++-
portal/flatpak-portal.c | 65 ++++++++++++++++++++++++++++++++++++++++-
2 files changed, 67 insertions(+), 2 deletions(-)
diff --git a/portal/Makefile.am.inc b/portal/Makefile.am.inc
index 119c1cf9..ef1f5da3 100644
--- a/portal/Makefile.am.inc
+++ b/portal/Makefile.am.inc
@@ -29,11 +29,13 @@ flatpak_portal_SOURCES = \
portal/flatpak-portal-app-info.h \
common/flatpak-portal-error.c \
common/flatpak-portal-error.h \
+ common/flatpak-utils-memfd.c \
+ common/flatpak-utils-memfd-private.h \
$(NULL)
BUILT_SOURCES += $(nodist_flatpak_portal_SOURCES)
CLEANFILES += $(nodist_flatpak_portal_SOURCES)
-flatpak_portal_LDADD = $(AM_LDADD) $(BASE_LIBS) $(OSTREE_LIBS)
+flatpak_portal_LDADD = $(AM_LDADD) $(BASE_LIBS) $(OSTREE_LIBS) libglnx.la
flatpak_portal_CFLAGS = $(AM_CFLAGS) $(BASE_CFLAGS) -DFLATPAK_COMPILATION
flatpak_portal_CPPFLAGS = $(AM_CPPFLAGS) -I$(builddir)/portal
diff --git a/portal/flatpak-portal.c b/portal/flatpak-portal.c
index ef52fc30..84338959 100644
--- a/portal/flatpak-portal.c
+++ b/portal/flatpak-portal.c
@@ -32,6 +32,14 @@
#include "flatpak-portal.h"
#include "flatpak-portal-app-info.h"
#include "flatpak-portal-error.h"
+#include "flatpak-utils-memfd-private.h"
+
+/* Syntactic sugar added in newer GLib, which makes the error paths more
+ * clearly correct */
+#ifndef G_DBUS_METHOD_INVOCATION_HANDLED
+# define G_DBUS_METHOD_INVOCATION_HANDLED TRUE
+# define G_DBUS_METHOD_INVOCATION_UNHANDLED FALSE
+#endif
#define IDLE_TIMEOUT_SECS 10 * 60
@@ -166,8 +174,15 @@ typedef struct
int fd_map_len;
gboolean set_tty;
int tty;
+ int env_fd;
} ChildSetupData;
+static void
+drop_cloexec (int fd)
+{
+ fcntl (fd, F_SETFD, 0);
+}
+
static void
child_setup_func (gpointer user_data)
{
@@ -176,6 +191,9 @@ child_setup_func (gpointer user_data)
sigset_t set;
int i;
+ if (data->env_fd != -1)
+ drop_cloexec (data->env_fd);
+
/* Unblock all signals */
sigemptyset (&set);
if (pthread_sigmask (SIG_SETMASK, &set, NULL) == -1)
@@ -321,6 +339,9 @@ handle_spawn (PortalFlatpak *object,
g_auto(GStrv) sandbox_expose = NULL;
g_auto(GStrv) sandbox_expose_ro = NULL;
gboolean sandboxed;
+ g_autoptr(GString) env_string = g_string_new ("");
+
+ child_setup_data.env_fd = -1;
app_info = g_object_get_data (G_OBJECT (invocation), "app-info");
g_assert (app_info != NULL);
@@ -505,7 +526,49 @@ handle_spawn (PortalFlatpak *object,
else
{
for (i = 0; extra_args != NULL && extra_args[i] != NULL; i++)
- g_ptr_array_add (flatpak_argv, g_strdup (extra_args[i]));
+ {
+ if (g_str_has_prefix (extra_args[i], "--env="))
+ {
+ const char *var_val = extra_args[i] + strlen ("--env=");
+
+ if (var_val[0] == '\0' || var_val[0] == '=')
+ {
+ g_warning ("Environment variable in extra-args has empty name");
+ continue;
+ }
+
+ if (strchr (var_val, '=') == NULL)
+ {
+ g_warning ("Environment variable in extra-args has no value");
+ continue;
+ }
+
+ g_string_append (env_string, var_val);
+ g_string_append_c (env_string, '\0');
+ }
+ else
+ {
+ g_ptr_array_add (flatpak_argv, g_strdup (extra_args[i]));
+ }
+ }
+ }
+
+ if (env_string->len > 0)
+ {
+ g_auto(GLnxTmpfile) env_tmpf = { 0, };
+
+ if (!flatpak_buffer_to_sealed_memfd_or_tmpfile (&env_tmpf, "environ",
+ env_string->str,
+ env_string->len, &error))
+ {
+ g_dbus_method_invocation_return_gerror (invocation, error);
+ return G_DBUS_METHOD_INVOCATION_HANDLED;
+ }
+
+ child_setup_data.env_fd = glnx_steal_fd (&env_tmpf.fd);
+ g_ptr_array_add (flatpak_argv,
+ g_strdup_printf ("--env-fd=%d",
+ child_setup_data.env_fd));
}
/* Inherit launcher network access from launcher, unless
--
2.29.2
From a83ebc5f82b83c7b4f26895769f4d1d3f0c083f6 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Sun, 10 Jan 2021 16:25:29 +0000
Subject: [PATCH 5/5] portal: Do not use caller-supplied variables in
environment
If the caller specifies a variable that can be used to inject arbitrary
code into processes, we must not allow it to enter the environment
block used to run `flatpak run`, which runs unsandboxed.
This change requires the previous commit "context: Add --env-fd option",
which adds infrastructure used here.
To be secure, this change also requires the previous commit
"run: Convert all environment variables into bwrap arguments", which
protects a non-setuid bwrap(1) from the same attack.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Part-of: https://github.com/flatpak/flatpak/security/advisories/GHSA-4ppf-fxf6-vxg2
---
portal/flatpak-portal.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/portal/flatpak-portal.c b/portal/flatpak-portal.c
index 84338959..ed2baf5c 100644
--- a/portal/flatpak-portal.c
+++ b/portal/flatpak-portal.c
@@ -506,6 +506,13 @@ 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++)
{
@@ -513,7 +520,26 @@ handle_spawn (PortalFlatpak *object,
const char *val = NULL;
g_variant_get_child (arg_envs, i, "{&s&s}", &var, &val);
- env = g_environ_setenv (env, var, val, TRUE);
+ 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"));
--
2.29.2