libminijailpreload: proper env cleaning
We add two utility functions in util.c:
- minijail_unsetenv, which removes a var from a given envp array
- minijail_getenv, which returns a var value from a given envp array
(same as getenv but working on an arbitrary array)
And a static utility function in util.c:
- getenv_index, which returns the index number of a passed variable
name, or the size of the passed array if not found
It's used by the above two utility functions, and has also been
integrated to minijail_setenv
And use them in libminijailpreload.c to:
- properly remove the minijail-injected .so in LD_PRELOAD, without
completely trashing any preexisting LD_PRELOAD, as was done before
in the code. A static func truncate_preload_env() only existing in
libminijailpreload.c is tasked to do this, and
- properly remove the __MINIJAIL_FD (kFdEnvVar) internal variable from
the child environment before spawning it. Before, this was done by
zeroing the proper envp entry, which is flagged as being an invalid
environment by some programs, such as the MongoDB official client:
Without minijail:
$ /tmp/m/bin/mongo --help >/dev/null; echo $?
0
$ env -i ./minijail0 /tmp/m/bin/mongo --help >/dev/null; echo $?
Failed global initialization: BadValue malformed environment block
libminijail[4045108]: child process 4045109 exited with status 1
1
With minijail, with this patch:
$ env -i ./minijail0 /tmp/m/bin/mongo --help >/dev/null; echo $?
0
This can also be seen using `env`:
Without minijail:
$ env -i /usr/bin/env | xxd
$
With minijail, before this patch:
$ env -i ./minijail0 /usr/bin/env | xxd
00000000: 0a0a ..
$
With minijail, with this patch:
$ env -i ./minijail0 /usr/bin/env | xxd
$
Change-Id: I79739a4c6f527c49a31ce31aaa9085fa574a25ee
diff --git a/libminijailpreload.c b/libminijailpreload.c
index a98b736..b5a3c75 100644
--- a/libminijailpreload.c
+++ b/libminijailpreload.c
@@ -11,6 +11,7 @@
#include "libminijail.h"
#include "libminijail-private.h"
+#include "util.h"
#include <dlfcn.h>
#include <stdio.h>
@@ -23,18 +24,24 @@
static int (*real_main) (int, char **, char **);
static void *libc_handle;
-static void die(const char *failed)
+static void truncate_preload_env(char **envp, const char *name)
{
- syslog(LOG_ERR, "libminijail: %s", failed);
- abort();
-}
-
-static void unset_in_env(char **envp, const char *name)
-{
- int i;
- for (i = 0; envp[i]; i++)
- if (!strncmp(envp[i], name, strlen(name)))
- envp[i][0] = '\0';
+ char *env_value = minijail_getenv(envp, name);
+ if (env_value) {
+ /*
+ * if we have more than just libminijailpreload.so in
+ * LD_PRELOAD, cut out libminijailpreload.so from it,
+ * as it is guaranteed to always be last in the
+ * LD_PRELOAD list.
+ */
+ char *last_space = strrchr(env_value, ' ');
+ if (last_space) {
+ *last_space = '\0';
+ } else {
+ /* Only our lib was in LD_PRELOAD, just unset it. */
+ minijail_unsetenv(envp, name);
+ }
+ }
}
/** @brief Fake main(), spliced in before the real call to main() by
@@ -71,12 +78,10 @@
die("preload: failed to parse minijail from parent");
close(fd);
- unset_in_env(envp, kFdEnvVar);
- /* TODO(ellyjones): this trashes existing preloads, so one can't do:
- * LD_PRELOAD="/tmp/test.so libminijailpreload.so" prog; the
- * descendants of prog will have no LD_PRELOAD set at all.
- */
- unset_in_env(envp, kLdPreloadEnvVar);
+ minijail_unsetenv(envp, kFdEnvVar);
+
+ truncate_preload_env(envp, kLdPreloadEnvVar);
+
/* Strip out flags meant for the parent. */
minijail_preenter(j);
minijail_enter(j);
