107 lines
3.3 KiB
Diff
107 lines
3.3 KiB
Diff
From bb7d8b1f41f7bf0399204d54009d6da57c3cc775 Mon Sep 17 00:00:00 2001
|
|
From: Christian Brauner <christian.brauner@ubuntu.com>
|
|
Date: Thu, 14 Feb 2019 15:56:26 +0100
|
|
Subject: [PATCH 50/50] nsexec (CVE-2019-5736): avoid parsing environ
|
|
|
|
My first attempt to simplify this and make it less costly focussed on
|
|
the way constructors are called. I was under the impression that the ELF
|
|
specification mandated that arg, argv, and actually even envp need to be
|
|
passed to functions located in the .init_arry section (aka
|
|
"constructors"). Actually, the specifications is (cf. [2]):
|
|
|
|
SHT_INIT_ARRAY
|
|
This section contains an array of pointers to initialization functions,
|
|
as described in ``Initialization and Termination Functions'' in Chapter
|
|
5. Each pointer in the array is taken as a parameterless procedure with
|
|
a void return.
|
|
|
|
which means that this becomes a libc specific decision. Glibc passes
|
|
down those args, musl doesn't. So this approach can't work. However, we
|
|
can at least remove the environment parsing part based on POSIX since
|
|
[1] mandates that there should be an environ variable defined in
|
|
unistd.h which provides access to the environment. See also the relevant
|
|
Open Group specification [1].
|
|
|
|
[1]: http://pubs.opengroup.org/onlinepubs/9699919799/
|
|
[2]: http://www.sco.com/developers/gabi/latest/ch4.sheader.html#init_array
|
|
|
|
Fixes: CVE-2019-5736
|
|
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
|
|
---
|
|
libcontainer/nsenter/cloned_binary.c | 23 ++++++++++-------------
|
|
1 file changed, 10 insertions(+), 13 deletions(-)
|
|
|
|
diff --git a/libcontainer/nsenter/cloned_binary.c b/libcontainer/nsenter/cloned_binary.c
|
|
index c8a42c23..c97dfcb7 100644
|
|
--- a/libcontainer/nsenter/cloned_binary.c
|
|
+++ b/libcontainer/nsenter/cloned_binary.c
|
|
@@ -169,31 +169,25 @@ static int parse_xargs(char *data, int data_length, char ***output)
|
|
}
|
|
|
|
/*
|
|
- * "Parse" out argv and envp from /proc/self/cmdline and /proc/self/environ.
|
|
+ * "Parse" out argv from /proc/self/cmdline.
|
|
* This is necessary because we are running in a context where we don't have a
|
|
* main() that we can just get the arguments from.
|
|
*/
|
|
-static int fetchve(char ***argv, char ***envp)
|
|
+static int fetchve(char ***argv)
|
|
{
|
|
- char *cmdline = NULL, *environ = NULL;
|
|
- size_t cmdline_size, environ_size;
|
|
+ char *cmdline = NULL;
|
|
+ size_t cmdline_size;
|
|
|
|
cmdline = read_file("/proc/self/cmdline", &cmdline_size);
|
|
if (!cmdline)
|
|
goto error;
|
|
- environ = read_file("/proc/self/environ", &environ_size);
|
|
- if (!environ)
|
|
- goto error;
|
|
|
|
if (parse_xargs(cmdline, cmdline_size, argv) <= 0)
|
|
goto error;
|
|
- if (parse_xargs(environ, environ_size, envp) <= 0)
|
|
- goto error;
|
|
|
|
return 0;
|
|
|
|
error:
|
|
- free(environ);
|
|
free(cmdline);
|
|
return -EINVAL;
|
|
}
|
|
@@ -246,23 +240,26 @@ error:
|
|
return -EIO;
|
|
}
|
|
|
|
+/* Get cheap access to the environment. */
|
|
+extern char **environ;
|
|
+
|
|
int ensure_cloned_binary(void)
|
|
{
|
|
int execfd;
|
|
- char **argv = NULL, **envp = NULL;
|
|
+ char **argv = NULL;
|
|
|
|
/* Check that we're not self-cloned, and if we are then bail. */
|
|
int cloned = is_self_cloned();
|
|
if (cloned > 0 || cloned == -ENOTRECOVERABLE)
|
|
return cloned;
|
|
|
|
- if (fetchve(&argv, &envp) < 0)
|
|
+ if (fetchve(&argv) < 0)
|
|
return -EINVAL;
|
|
|
|
execfd = clone_binary();
|
|
if (execfd < 0)
|
|
return -EIO;
|
|
|
|
- fexecve(execfd, argv, envp);
|
|
+ fexecve(execfd, argv, environ);
|
|
return -ENOEXEC;
|
|
}
|
|
--
|
|
2.17.1
|
|
|