From 1c7ea15b30d49d32a0e3cb79152514f1aeb19397 Mon Sep 17 00:00:00 2001 From: Topi Miettinen Date: Mon, 6 Apr 2020 21:24:16 +0300 Subject: Filter environment variables Save all environment variables for later use in the application, clear environment and re-apply only whitelisted variables for the main firejail process. The whitelisted environment is only used by C library. Sandboxed tools will get further variables used internally (FIREJAIL_*). All variables will be reapplied for the firejailed application. This also lifts the length restriction for environment variables, except for the variables used by Firejail itself or the sandboxed tools. --- src/firejail/appimage.c | 18 ++--- src/firejail/checkcfg.c | 6 +- src/firejail/chroot.c | 4 +- src/firejail/dbus.c | 30 +++----- src/firejail/env.c | 164 ++++++++++++++++++++++++++++++++++++-------- src/firejail/firejail.h | 11 ++- src/firejail/fs.c | 2 +- src/firejail/fs_whitelist.c | 2 +- src/firejail/join.c | 2 +- src/firejail/main.c | 53 ++++++++------ src/firejail/no_sandbox.c | 2 +- src/firejail/output.c | 7 ++ src/firejail/paths.c | 10 +-- src/firejail/profile.c | 10 +-- src/firejail/pulseaudio.c | 5 +- src/firejail/run_symlink.c | 7 +- src/firejail/sandbox.c | 7 +- src/firejail/sbox.c | 2 +- src/firejail/x11.c | 61 ++++++++++++---- 19 files changed, 284 insertions(+), 119 deletions(-) diff --git a/src/firejail/appimage.c b/src/firejail/appimage.c index 6190b6f01..dd94b9921 100644 --- a/src/firejail/appimage.c +++ b/src/firejail/appimage.c @@ -131,14 +131,16 @@ void appimage_set(const char *appimage) { errExit("Failed to obtain absolute path"); // set environment - if (setenv("APPIMAGE", abspath, 1) < 0) - errExit("setenv"); - if (mntdir && setenv("APPDIR", mntdir, 1) < 0) - errExit("setenv"); - if (size != 0 && setenv("ARGV0", appimage, 1) < 0) - errExit("setenv"); - if (cfg.cwd && setenv("OWD", cfg.cwd, 1) < 0) - errExit("setenv"); + env_store_name_val("APPIMAGE", abspath, SETENV); + + if (mntdir) + env_store_name_val("APPDIR", mntdir, SETENV); + + if (size != 0) + env_store_name_val("ARGV0", appimage, SETENV); + + if (cfg.cwd) + env_store_name_val("OWD", cfg.cwd, SETENV); // build new command line if (asprintf(&cfg.command_line, "%s/AppRun", mntdir) == -1) diff --git a/src/firejail/checkcfg.c b/src/firejail/checkcfg.c index 085221464..fb2171a55 100644 --- a/src/firejail/checkcfg.c +++ b/src/firejail/checkcfg.c @@ -215,10 +215,8 @@ int checkcfg(int val) { } // file copy limit - else if (strncmp(ptr, "file-copy-limit ", 16) == 0) { - if (setenv("FIREJAIL_FILE_COPY_LIMIT", ptr + 16, 1) == -1) - errExit("setenv"); - } + else if (strncmp(ptr, "file-copy-limit ", 16) == 0) + env_store_name_val("FIREJAIL_FILE_COPY_LIMIT", ptr + 16, SETENV); // timeout for join option else if (strncmp(ptr, "join-timeout ", 13) == 0) diff --git a/src/firejail/chroot.c b/src/firejail/chroot.c index cfa32d1d3..6de4b819c 100644 --- a/src/firejail/chroot.c +++ b/src/firejail/chroot.c @@ -173,7 +173,7 @@ void fs_chroot(const char *rootdir) { // x11 // if users want this mount, they should set FIREJAIL_CHROOT_X11 - if (getenv("FIREJAIL_X11") || getenv("FIREJAIL_CHROOT_X11")) { + if (env_get("FIREJAIL_X11") || env_get("FIREJAIL_CHROOT_X11")) { if (arg_debug) printf("Mounting /tmp/.X11-unix on chroot /tmp/.X11-unix\n"); check_subdir(parentfd, "tmp/.X11-unix", 0); @@ -194,7 +194,7 @@ void fs_chroot(const char *rootdir) { check_subdir(parentfd, "run", 1); // pulseaudio; only support for default directory /run/user/$UID/pulse - if (getenv("FIREJAIL_CHROOT_PULSE")) { + if (env_get("FIREJAIL_CHROOT_PULSE")) { char *pulse; if (asprintf(&pulse, "%s/run/user/%d/pulse", cfg.chrootdir, getuid()) == -1) errExit("asprintf"); diff --git a/src/firejail/dbus.c b/src/firejail/dbus.c index 3cf75ed84..1d0f07089 100644 --- a/src/firejail/dbus.c +++ b/src/firejail/dbus.c @@ -329,7 +329,7 @@ void dbus_proxy_start(void) { errExit("close"); if (arg_dbus_user == DBUS_POLICY_FILTER) { - char *user_env = getenv(DBUS_SESSION_BUS_ADDRESS_ENV); + const char *user_env = env_get(DBUS_SESSION_BUS_ADDRESS_ENV); if (user_env == NULL) { char *dbus_user_socket = find_user_socket(); write_arg(args_pipe[1], DBUS_SOCKET_PATH_PREFIX "%s", @@ -350,7 +350,7 @@ void dbus_proxy_start(void) { } if (arg_dbus_system == DBUS_POLICY_FILTER) { - char *system_env = getenv(DBUS_SYSTEM_BUS_ADDRESS_ENV); + const char *system_env = env_get(DBUS_SYSTEM_BUS_ADDRESS_ENV); if (system_env == NULL) { write_arg(args_pipe[1], DBUS_SOCKET_PATH_PREFIX DBUS_SYSTEM_SOCKET); @@ -435,8 +435,8 @@ static void socket_overlay(char *socket_path, char *proxy_path) { close(fd); } -static char *get_socket_env(const char *name) { - char *value = getenv(name); +static const char *get_socket_env(const char *name) { + const char *value = env_get(name); if (value == NULL) return NULL; if (strncmp(value, DBUS_SOCKET_PATH_PREFIX, @@ -446,21 +446,13 @@ static char *get_socket_env(const char *name) { } void dbus_set_session_bus_env(void) { - if (setenv(DBUS_SESSION_BUS_ADDRESS_ENV, - DBUS_SOCKET_PATH_PREFIX RUN_DBUS_USER_SOCKET, 1) == -1) { - fprintf(stderr, "Error: cannot modify " DBUS_SESSION_BUS_ADDRESS_ENV - " required by --dbus-user\n"); - exit(1); - } + env_store_name_val(DBUS_SESSION_BUS_ADDRESS_ENV, + DBUS_SOCKET_PATH_PREFIX RUN_DBUS_USER_SOCKET, SETENV); } void dbus_set_system_bus_env(void) { - if (setenv(DBUS_SYSTEM_BUS_ADDRESS_ENV, - DBUS_SOCKET_PATH_PREFIX RUN_DBUS_SYSTEM_SOCKET, 1) == -1) { - fprintf(stderr, "Error: cannot modify " DBUS_SYSTEM_BUS_ADDRESS_ENV - " required by --dbus-system\n"); - exit(1); - } + env_store_name_val(DBUS_SYSTEM_BUS_ADDRESS_ENV, + DBUS_SOCKET_PATH_PREFIX RUN_DBUS_SYSTEM_SOCKET, SETENV); } static void disable_socket_dir(void) { @@ -506,7 +498,7 @@ void dbus_apply_policy(void) { errExit("asprintf"); disable_file_or_dir(dbus_user_socket2); - char *user_env = get_socket_env(DBUS_SESSION_BUS_ADDRESS_ENV); + const char *user_env = get_socket_env(DBUS_SESSION_BUS_ADDRESS_ENV); if (user_env != NULL && strcmp(user_env, dbus_user_socket) != 0 && strcmp(user_env, dbus_user_socket2) != 0) disable_file_or_dir(user_env); @@ -535,7 +527,7 @@ void dbus_apply_policy(void) { disable_file_or_dir(DBUS_SYSTEM_SOCKET); - char *system_env = get_socket_env(DBUS_SYSTEM_BUS_ADDRESS_ENV); + const char *system_env = get_socket_env(DBUS_SYSTEM_BUS_ADDRESS_ENV); if (system_env != NULL && strcmp(system_env, DBUS_SYSTEM_SOCKET) != 0) disable_file_or_dir(system_env); @@ -561,4 +553,4 @@ void dbus_apply_policy(void) { fwarning("An abstract unix socket for session D-BUS might still be available. Use --net or remove unix from --protocol set.\n"); } -#endif // HAVE_DBUSPROXY \ No newline at end of file +#endif // HAVE_DBUSPROXY diff --git a/src/firejail/env.c b/src/firejail/env.c index d74cebb39..2d5042100 100644 --- a/src/firejail/env.c +++ b/src/firejail/env.c @@ -25,8 +25,8 @@ typedef struct env_t { struct env_t *next; - char *name; - char *value; + const char *name; + const char *value; ENV_OP op; } Env; static Env *envlist = NULL; @@ -117,45 +117,35 @@ void env_ibus_load(void) { // default sandbox env variables void env_defaults(void) { // Qt fixes - if (setenv("QT_X11_NO_MITSHM", "1", 1) < 0) - errExit("setenv"); - if (setenv("QML_DISABLE_DISK_CACHE", "1", 1) < 0) - errExit("setenv"); -// if (setenv("QTWEBENGINE_DISABLE_SANDBOX", "1", 1) < 0) -// errExit("setenv"); -// if (setenv("MOZ_NO_REMOTE, "1", 1) < 0) -// errExit("setenv"); - if (setenv("container", "firejail", 1) < 0) // LXC sets container=lxc, - errExit("setenv"); + env_store_name_val("QT_X11_NO_MITSHM", "1", SETENV); + env_store_name_val("QML_DISABLE_DISK_CACHE", "1", SETENV); +// env_store_name_val("QTWEBENGINE_DISABLE_SANDBOX", "1", SETENV); +// env_store_name_val("MOZ_NO_REMOTE, "1", SETENV); + env_store_name_val("container", "firejail", SETENV); // LXC sets container=lxc, if (!cfg.shell) cfg.shell = guess_shell(); - if (cfg.shell && setenv("SHELL", cfg.shell, 1) < 0) - errExit("setenv"); + if (cfg.shell) + env_store_name_val("SHELL", cfg.shell, SETENV); // spawn KIO slaves inside the sandbox - if (setenv("KDE_FORK_SLAVES", "1", 1) < 0) - errExit("setenv"); + env_store_name_val("KDE_FORK_SLAVES", "1", SETENV); // set prompt color to green int set_prompt = 0; if (checkcfg(CFG_FIREJAIL_PROMPT)) set_prompt = 1; else { // check FIREJAIL_PROMPT="yes" environment variable - char *prompt = getenv("FIREJAIL_PROMPT"); + const char *prompt = env_get("FIREJAIL_PROMPT"); if (prompt && strcmp(prompt, "yes") == 0) set_prompt = 1; } - if (set_prompt) { + if (set_prompt) //export PS1='\[\e[1;32m\][\u@\h \W]\$\[\e[0m\] ' - if (setenv("PROMPT_COMMAND", "export PS1=\"\\[\\e[1;32m\\][\\u@\\h \\W]\\$\\[\\e[0m\\] \"", 1) < 0) - errExit("setenv"); - } - else { + env_store_name_val("PROMPT_COMMAND", "export PS1=\"\\[\\e[1;32m\\][\\u@\\h \\W]\\$\\[\\e[0m\\] \"", SETENV); + else // remove PROMPT_COMMAND - if (setenv("PROMPT_COMMAND", ":", 1) < 0) // unsetenv() will not work here, bash still picks it up from somewhere - errExit("setenv"); - } + env_store_name_val("PROMPT_COMMAND", ":", SETENV); // unsetenv() will not work here, bash still picks it up from somewhere // set the window title if (!arg_quiet && isatty(STDOUT_FILENO)) @@ -163,26 +153,26 @@ void env_defaults(void) { // pass --quiet as an environment variable, in case the command calls further firejailed commands if (arg_quiet) - setenv("FIREJAIL_QUIET", "yes", 1); + env_store_name_val("FIREJAIL_QUIET", "yes", SETENV); fflush(0); } // parse and store the environment setting void env_store(const char *str, ENV_OP op) { - EUID_ASSERT(); assert(str); // some basic checking if (*str == '\0') goto errexit; char *ptr = strchr(str, '='); - if (op == SETENV) { + if (op == SETENV || op == SETENV_ALLOW_EMPTY) { if (!ptr) goto errexit; ptr++; - if (*ptr == '\0') + if (*ptr == '\0' && op != SETENV_ALLOW_EMPTY) goto errexit; + op = SETENV; } // build list entry @@ -210,8 +200,45 @@ errexit: exit(1); } +void env_store_name_val(const char *name, const char *val, ENV_OP op) { + assert(name); + + // some basic checking + if (*name == '\0') + goto errexit; + if (*val == '\0' && op != SETENV_ALLOW_EMPTY) + goto errexit; + + if (op == SETENV_ALLOW_EMPTY) + op = SETENV; + + // build list entry + Env *env = calloc(1, sizeof(Env)); + if (!env) + errExit("calloc"); + + env->name = strdup(name); + if (env->name == NULL) + errExit("strdup"); + + if (op == SETENV) { + env->value = strdup(val); + if (env->value == NULL) + errExit("strdup"); + } + env->op = op; + + // add entry to the list + env_add(env); + return; + +errexit: + fprintf(stderr, "Error: invalid --env setting\n"); + exit(1); +} + // set env variables in the new sandbox process -void env_apply(void) { +void env_apply_all(void) { Env *env = envlist; while (env) { @@ -225,3 +252,80 @@ void env_apply(void) { env = env->next; } } + +// get env variable +const char *env_get(const char *name) { + Env *env = envlist; + const char *r = NULL; + + while (env) { + if (strcmp(env->name, name) == 0) { + if (env->op == SETENV) + r = env->value; + else if (env->op == RMENV) + r = NULL; + } + env = env->next; + } + return r; +} + +static const char * const env_whitelist[] = { + "LANG", + "LANGUAGE", + "LC_MESSAGES", + "PATH" +}; + +static const char * const env_whitelist_sbox[] = { + "FIREJAIL_DEBUG", + "FIREJAIL_FILE_COPY_LIMIT", + "FIREJAIL_PLUGIN", + "FIREJAIL_QUIET", + "FIREJAIL_SECCOMP_ERROR_ACTION", + "FIREJAIL_TEST_ARGUMENTS", + "FIREJAIL_TRACEFILE" +}; + +static void env_apply_list(const char * const *list, unsigned int num_items) { + Env *env = envlist; + + while (env) { + if (env->op == SETENV) { + for (unsigned int i = 0; i < num_items; i++) + if (strcmp(env->name, list[i]) == 0) { + // sanity check for whitelisted environment variables + if (strlen(env->name) + strlen(env->value) >= MAX_ENV_LEN) { + fprintf(stderr, "Error: too long environment variable %s, please use --rmenv\n", env->name); + exit(1); + } + + //fprintf(stderr, "whitelisted env var %s=%s\n", env->name, env->value); + if (setenv(env->name, env->value, 1) < 0) + errExit("setenv"); + break; + } + } else if (env->op == RMENV) + unsetenv(env->name); + + env = env->next; + } +} + +// Filter env variables in main firejail process. All variables will +// be reapplied for the sandboxed app by env_apply_all(). +void env_apply_whitelist(void) { + int r; + + r = clearenv(); + if (r != 0) + errExit("clearenv"); + + env_apply_list(env_whitelist, ARRAY_SIZE(env_whitelist)); +} + +// Filter env variables for a sbox app +void env_apply_whitelist_sbox(void) { + env_apply_whitelist(); + env_apply_list(env_whitelist_sbox, ARRAY_SIZE(env_whitelist_sbox)); +} diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index 9ea3edcd0..c6e0fed2a 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -81,6 +81,8 @@ (void) rv;\ } while (0) +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + // main.c typedef struct bridge_t { // on the host @@ -655,16 +657,21 @@ int check_kernel_procs(void); void run_no_sandbox(int argc, char **argv) __attribute__((noreturn)); #define MAX_ENVS 256 // some sane maximum number of environment variables -#define MAX_ENV_LEN (PATH_MAX + 32) // FOOBAR=SOME_PATH +#define MAX_ENV_LEN (PATH_MAX + 32) // FOOBAR=SOME_PATH, only applied to Firejail's own sandboxed apps // env.c typedef enum { SETENV = 0, + SETENV_ALLOW_EMPTY, RMENV } ENV_OP; void env_store(const char *str, ENV_OP op); -void env_apply(void); +void env_store_name_val(const char *name, const char *val, ENV_OP op); +void env_apply_all(void); +void env_apply_whitelist(void); +void env_apply_whitelist_sbox(void); void env_defaults(void); +const char *env_get(const char *name); void env_ibus_load(void); // fs_whitelist.c diff --git a/src/firejail/fs.c b/src/firejail/fs.c index 4e6c1adc3..b1c509b30 100644 --- a/src/firejail/fs.c +++ b/src/firejail/fs.c @@ -1221,7 +1221,7 @@ void fs_private_tmp(void) { printf("Generate private-tmp whitelist commands\n"); // check XAUTHORITY file, KDE keeps it under /tmp - char *xauth = getenv("XAUTHORITY"); + const char *xauth = env_get("XAUTHORITY"); if (xauth) { char *rp = realpath(xauth, NULL); if (rp && strncmp(rp, "/tmp/", 5) == 0) { diff --git a/src/firejail/fs_whitelist.c b/src/firejail/fs_whitelist.c index 1d7552339..d60c57fec 100644 --- a/src/firejail/fs_whitelist.c +++ b/src/firejail/fs_whitelist.c @@ -778,7 +778,7 @@ void fs_whitelist(void) { fs_logger("tmpfs /tmp"); // pam-tmpdir - issue #2685 - char *env = getenv("TMP"); + const char *env = env_get("TMP"); if (env) { char *pamtmpdir; if (asprintf(&pamtmpdir, "/tmp/user/%u", getuid()) == -1) diff --git a/src/firejail/join.c b/src/firejail/join.c index 4f0210f95..bdd0f286c 100644 --- a/src/firejail/join.c +++ b/src/firejail/join.c @@ -561,7 +561,7 @@ void join(pid_t pid, int argc, char **argv, int index) { char *display_str; if (asprintf(&display_str, ":%d", display) == -1) errExit("asprintf"); - setenv("DISPLAY", display_str, 1); + env_store_name_val("DISPLAY", display_str, SETENV); free(display_str); } diff --git a/src/firejail/main.c b/src/firejail/main.c index 0f0086a6e..7a9521e42 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -861,19 +861,20 @@ static void run_cmd_and_exit(int i, int argc, char **argv) { } char *guess_shell(void) { - char *shell = NULL; + const char *shell; + char *retval; struct stat s; - shell = getenv("SHELL"); + shell = env_get("SHELL"); if (shell) { invalid_filename(shell, 0); // no globbing if (!is_dir(shell) && strstr(shell, "..") == NULL && stat(shell, &s) == 0 && access(shell, X_OK) == 0 && strcmp(shell, PATH_FIREJAIL) != 0) - return shell; + goto found; } // shells in order of preference - char *shells[] = {"/bin/bash", "/bin/csh", "/usr/bin/zsh", "/bin/sh", "/bin/ash", NULL }; + static const char * const shells[] = {"/bin/bash", "/bin/csh", "/usr/bin/zsh", "/bin/sh", "/bin/ash", NULL }; int i = 0; while (shells[i] != NULL) { @@ -884,8 +885,11 @@ char *guess_shell(void) { } i++; } - - return shell; + found: + retval = strdup(shell); + if (!retval) + errExit("strdup"); + return retval; } // return argument index @@ -926,9 +930,13 @@ static void run_builder(int argc, char **argv) { if (setresuid(-1, getuid(), getuid()) != 0) errExit("setresuid"); + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); umask(orig_umask); + // restore some environment variables + env_apply_whitelist_sbox(); + argv[0] = LIBDIR "/firejail/fbuilder"; execvp(argv[0], argv); @@ -994,6 +1002,16 @@ int main(int argc, char **argv, char **envp) { exit(1); } + // Stash environment variables + for (i = 0, ptr = envp; ptr && *ptr && i < MAX_ENVS; i++, ptr++) + env_store(*ptr, SETENV_ALLOW_EMPTY); + + // sanity check for environment variables + if (i >= MAX_ENVS) { + fprintf(stderr, "Error: too many environment variables, please use --rmenv\n"); + exit(1); + } + // sanity check for arguments for (i = 0; i < argc; i++) { if (*argv[i] == 0) { @@ -1005,29 +1023,19 @@ int main(int argc, char **argv, char **envp) { exit(1); } // Also remove requested environment variables - // entirely to avoid tripping the length check below if (strncmp(argv[i], "--rmenv=", 8) == 0) - unsetenv(argv[i] + 8); + env_store(argv[i] + 8, RMENV); } - // sanity check for environment variables - for (i = 0, ptr = envp; ptr && *ptr && i < MAX_ENVS; i++, ptr++) { - if (strlen(*ptr) >= MAX_ENV_LEN) { - fprintf(stderr, "Error: too long environment variables, please use --rmenv\n"); - exit(1); - } - } - if (i >= MAX_ENVS) { - fprintf(stderr, "Error: too many environment variables, please use --rmenv\n"); - exit(1); - } + // Reapply a minimal set of environment variables + env_apply_whitelist(); // check if the user is allowed to use firejail init_cfg(argc, argv); // get starting timestamp, process --quiet timetrace_start(); - char *env_quiet = getenv("FIREJAIL_QUIET"); + const char *env_quiet = env_get("FIREJAIL_QUIET"); if (check_arg(argc, argv, "--quiet", 1) || (env_quiet && strcmp(env_quiet, "yes") == 0)) arg_quiet = 1; @@ -1037,7 +1045,7 @@ int main(int argc, char **argv, char **envp) { // build /run/firejail directory structure preproc_build_firejail_dir(); - char *container_name = getenv("container"); + const char *container_name = env_get("container"); if (!container_name || strcmp(container_name, "firejail")) { lockfd_directory = open(RUN_DIRECTORY_LOCK_FILE, O_WRONLY | O_CREAT, S_IRUSR | S_IWUSR); if (lockfd_directory != -1) { @@ -1170,6 +1178,9 @@ int main(int argc, char **argv, char **envp) { drop_privs(1); umask(orig_umask); + + // restore original environment variables + env_apply_all(); int rv = system(argv[2]); exit(rv); } diff --git a/src/firejail/no_sandbox.c b/src/firejail/no_sandbox.c index 6c7803602..111d94333 100644 --- a/src/firejail/no_sandbox.c +++ b/src/firejail/no_sandbox.c @@ -41,7 +41,7 @@ int check_namespace_virt(void) { EUID_ASSERT(); // check container environment variable - char *str = getenv("container"); + const char *str = env_get("container"); if (str && is_container(str)) return 1; diff --git a/src/firejail/output.c b/src/firejail/output.c index 36cb905cb..1682ee025 100644 --- a/src/firejail/output.c +++ b/src/firejail/output.c @@ -95,6 +95,9 @@ void check_output(int argc, char **argv) { close(pipefd[0]); } + // restore some environment variables + env_apply_whitelist_sbox(); + char *args[3]; args[0] = LIBDIR "/firejail/ftee"; args[1] = outfile; @@ -137,6 +140,10 @@ void check_output(int argc, char **argv) { } args[j++] = argv[i]; } + + // restore original environment variables + env_apply_all(); + execvp(args[0], args); perror("execvp"); diff --git a/src/firejail/paths.c b/src/firejail/paths.c index 5de704bef..981a6bc71 100644 --- a/src/firejail/paths.c +++ b/src/firejail/paths.c @@ -26,13 +26,13 @@ static unsigned int longest_path_elt = 0; static char *elt = NULL; // moved from inside init_paths in order to get rid of scan-build warning static void init_paths(void) { - char *path = getenv("PATH"); + const char *env_path = env_get("PATH"); char *p; - if (!path) { - path = "/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin"; - setenv("PATH", path, 1); + if (!env_path) { + env_path = "/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin"; + env_store_name_val("PATH", env_path, SETENV); } - path = strdup(path); + char *path = strdup(env_path); if (!path) errExit("strdup"); diff --git a/src/firejail/profile.c b/src/firejail/profile.c index 1ee8cdfcb..3766ba8f0 100644 --- a/src/firejail/profile.c +++ b/src/firejail/profile.c @@ -158,7 +158,7 @@ static int check_nosound(void) { } static int check_x11(void) { - return (arg_x11_block || arg_x11_xorg || getenv("FIREJAIL_X11")); + return (arg_x11_block || arg_x11_xorg || env_get("FIREJAIL_X11")); } static int check_disable_u2f(void) { @@ -1181,7 +1181,7 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { if (strcmp(ptr, "x11 xephyr") == 0) { #ifdef HAVE_X11 if (checkcfg(CFG_X11)) { - char *x11env = getenv("FIREJAIL_X11"); + const char *x11env = env_get("FIREJAIL_X11"); if (x11env && strcmp(x11env, "yes") == 0) { return 0; } @@ -1210,7 +1210,7 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { if (strcmp(ptr, "x11 xpra") == 0) { #ifdef HAVE_X11 if (checkcfg(CFG_X11)) { - char *x11env = getenv("FIREJAIL_X11"); + const char *x11env = env_get("FIREJAIL_X11"); if (x11env && strcmp(x11env, "yes") == 0) { return 0; } @@ -1229,7 +1229,7 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { if (strcmp(ptr, "x11 xvfb") == 0) { #ifdef HAVE_X11 if (checkcfg(CFG_X11)) { - char *x11env = getenv("FIREJAIL_X11"); + const char *x11env = env_get("FIREJAIL_X11"); if (x11env && strcmp(x11env, "yes") == 0) { return 0; } @@ -1248,7 +1248,7 @@ int profile_check_line(char *ptr, int lineno, const char *fname) { if (strcmp(ptr, "x11") == 0) { #ifdef HAVE_X11 if (checkcfg(CFG_X11)) { - char *x11env = getenv("FIREJAIL_X11"); + const char *x11env = env_get("FIREJAIL_X11"); if (x11env && strcmp(x11env, "yes") == 0) { return 0; } diff --git a/src/firejail/pulseaudio.c b/src/firejail/pulseaudio.c index a5c924a70..5df3d9cd3 100644 --- a/src/firejail/pulseaudio.c +++ b/src/firejail/pulseaudio.c @@ -42,7 +42,7 @@ void pulseaudio_disable(void) { // blacklist pulseaudio socket in XDG_RUNTIME_DIR - char *name = getenv("XDG_RUNTIME_DIR"); + const char *name = env_get("XDG_RUNTIME_DIR"); if (name) disable_file_path(name, "pulse/native"); @@ -76,7 +76,10 @@ void pulseaudio_disable(void) { } static void pulseaudio_fallback(const char *path) { + assert(path); + fmessage("Cannot mount tmpfs on %s/.config/pulse\n", cfg.homedir); + env_store_name_val("PULSE_CLIENTCONFIG", path, SETENV); if (setenv("PULSE_CLIENTCONFIG", path, 1) < 0) errExit("setenv"); } diff --git a/src/firejail/run_symlink.c b/src/firejail/run_symlink.c index b38cc0ca6..5bf27fc6d 100644 --- a/src/firejail/run_symlink.c +++ b/src/firejail/run_symlink.c @@ -42,7 +42,8 @@ void run_symlink(int argc, char **argv, int run_as_is) { errExit("setresuid"); // find the real program by looking in PATH - if (!getenv("PATH")) { + const char *path = env_get("PATH"); + if (!path) { fprintf(stderr, "Error: PATH environment variable not set\n"); exit(1); } @@ -57,6 +58,9 @@ void run_symlink(int argc, char **argv, int run_as_is) { // restore original umask umask(orig_umask); + // restore original environment variables + env_apply_all(); + // desktop integration is not supported for root user; instead, the original program is started if (getuid() == 0 || run_as_is) { argv[0] = program; @@ -73,6 +77,7 @@ void run_symlink(int argc, char **argv, int run_as_is) { a[i + 2] = argv[i + 1]; } a[i + 2] = NULL; + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(a[0], a); diff --git a/src/firejail/sandbox.c b/src/firejail/sandbox.c index d811fe45a..1f94d86cd 100644 --- a/src/firejail/sandbox.c +++ b/src/firejail/sandbox.c @@ -268,8 +268,7 @@ static void sandbox_if_up(Bridge *br) { static void chk_chroot(void) { // if we are starting firejail inside some other container technology, we don't care about this - char *mycont = getenv("container"); - if (mycont) + if (env_get("container")) return; // check if this is a regular chroot @@ -419,7 +418,7 @@ static int ok_to_run(const char *program) { return 1; } else { // search $PATH - char *path1 = getenv("PATH"); + const char *path1 = env_get("PATH"); if (path1) { if (arg_debug) printf("Searching $PATH for %s\n", program); @@ -465,7 +464,7 @@ void start_application(int no_sandbox, int fd, char *set_sandbox_status) { // set environment if (no_sandbox == 0) { env_defaults(); - env_apply(); + env_apply_all(); } // restore original umask umask(orig_umask); diff --git a/src/firejail/sbox.c b/src/firejail/sbox.c index c3d3bd72c..baf99c5b9 100644 --- a/src/firejail/sbox.c +++ b/src/firejail/sbox.c @@ -36,7 +36,7 @@ static int __attribute__((noreturn)) sbox_do_exec_v(unsigned filtermask, char * int env_index = 0; char *new_environment[256] = { NULL }; // preserve firejail-specific env vars - char *cl = getenv("FIREJAIL_FILE_COPY_LIMIT"); + const char *cl = env_get("FIREJAIL_FILE_COPY_LIMIT"); if (cl) { if (asprintf(&new_environment[env_index++], "FIREJAIL_FILE_COPY_LIMIT=%s", cl) == -1) errExit("asprintf"); diff --git a/src/firejail/x11.c b/src/firejail/x11.c index 4872a5207..1121ec84e 100644 --- a/src/firejail/x11.c +++ b/src/firejail/x11.c @@ -41,7 +41,7 @@ // Parse the DISPLAY environment variable and return a display number. // Returns -1 if DISPLAY is not set, or is set to anything other than :ddd. int x11_display(void) { - const char *display_str = getenv("DISPLAY"); + const char *display_str = env_get("DISPLAY"); char *endp; unsigned long display; @@ -208,7 +208,7 @@ void x11_start_xvfb(int argc, char **argv) { pid_t jail = 0; pid_t server = 0; - setenv("FIREJAIL_X11", "yes", 1); + env_store_name_val("FIREJAIL_X11", "yes", SETENV); // never try to run X servers as root!!! if (getuid() == 0) { @@ -326,7 +326,11 @@ void x11_start_xvfb(int argc, char **argv) { if (arg_debug) printf("Starting xvfb...\n"); + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(server_argv[0], server_argv); perror("execvp"); @@ -355,7 +359,7 @@ void x11_start_xvfb(int argc, char **argv) { free(fname); assert(display_str); - setenv("DISPLAY", display_str, 1); + env_store_name_val("DISPLAY", display_str, SETENV); // run attach command jail = fork(); if (jail < 0) @@ -363,7 +367,11 @@ void x11_start_xvfb(int argc, char **argv) { if (jail == 0) { fmessage("\n*** Attaching to Xvfb display %d ***\n\n", display); + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(jail_argv[0], jail_argv); perror("execvp"); @@ -428,7 +436,7 @@ void x11_start_xephyr(int argc, char **argv) { if (newscreen) xephyr_screen = newscreen; - setenv("FIREJAIL_X11", "yes", 1); + env_store_name_val("FIREJAIL_X11", "yes", SETENV); // unfortunately, xephyr does a number of weird things when started by root user!!! if (getuid() == 0) { @@ -556,7 +564,11 @@ void x11_start_xephyr(int argc, char **argv) { if (arg_debug) printf("Starting xephyr...\n"); + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(server_argv[0], server_argv); perror("execvp"); @@ -585,7 +597,7 @@ void x11_start_xephyr(int argc, char **argv) { free(fname); assert(display_str); - setenv("DISPLAY", display_str, 1); + env_store_name_val("DISPLAY", display_str, SETENV); // run attach command jail = fork(); if (jail < 0) @@ -594,8 +606,12 @@ void x11_start_xephyr(int argc, char **argv) { if (!arg_quiet) printf("\n*** Attaching to Xephyr display %d ***\n\n", display); + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above assert(getenv("LD_PRELOAD") == NULL); + assert(env_get("LD_PRELOAD") == NULL); execvp(jail_argv[0], jail_argv); perror("execvp"); _exit(1); @@ -780,8 +796,12 @@ static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, dup2(fd_null,2); } + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above assert(getenv("LD_PRELOAD") == NULL); + assert(env_get("LD_PRELOAD") == NULL); execvp(server_argv[0], server_argv); perror("execvp"); _exit(1); @@ -827,7 +847,11 @@ static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, fmessage("\n*** Attaching to xpra display %d ***\n\n", display); + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(attach_argv[0], attach_argv); perror("execvp"); @@ -835,7 +859,7 @@ static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, } assert(display_str); - setenv("DISPLAY", display_str, 1); + env_store_name_val("DISPLAY", display_str, SETENV); // build jail command char *firejail_argv[argc+2]; @@ -857,7 +881,12 @@ static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, errExit("fork"); if (jail == 0) { // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); + + // restore original environment variables + env_apply_all(); + if (firejail_argv[0]) // shut up llvm scan-build execvp(firejail_argv[0], firejail_argv); perror("execvp"); @@ -883,7 +912,12 @@ static void __attribute__((noreturn)) x11_start_xpra_old(int argc, char **argv, dup2(fd_null,1); dup2(fd_null,2); } + + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(stop_argv[0], stop_argv); perror("execvp"); @@ -1051,7 +1085,11 @@ static void __attribute__((noreturn)) x11_start_xpra_new(int argc, char **argv, dup2(fd_null,2); } + // restore original environment variables + env_apply_all(); + // running without privileges - see drop_privs call above + assert(env_get("LD_PRELOAD") == NULL); assert(getenv("LD_PRELOAD") == NULL); execvp(server_argv[0], server_argv); perror("execvp"); @@ -1072,7 +1110,7 @@ static void __attribute__((noreturn)) x11_start_xpra_new(int argc, char **argv, void x11_start_xpra(int argc, char **argv) { EUID_ASSERT(); - setenv("FIREJAIL_X11", "yes", 1); + env_store_name_val("FIREJAIL_X11", "yes", SETENV); // unfortunately, xpra does a number of weird things when started by root user!!! if (getuid() == 0) { @@ -1134,7 +1172,7 @@ void x11_xorg(void) { #ifdef HAVE_X11 // get DISPLAY env - char *display = getenv("DISPLAY"); + const char *display = env_get("DISPLAY"); if (!display) { fputs("Error: --x11=xorg requires an 'outer' X11 server to use.\n", stderr); exit(1); @@ -1259,7 +1297,7 @@ void x11_xorg(void) { ASSERT_PERMS(dest, getuid(), getgid(), 0600); // blacklist user .Xauthority file if it is not masked already - char *envar = getenv("XAUTHORITY"); + const char *envar = env_get("XAUTHORITY"); if (envar) { char *rp = realpath(envar, NULL); if (rp) { @@ -1269,8 +1307,7 @@ void x11_xorg(void) { } } // set environment variable - if (setenv("XAUTHORITY", dest, 1) < 0) - errExit("setenv"); + env_store_name_val("XAUTHORITY", dest, SETENV); free(dest); // mask RUN_XAUTHORITY_SEC_DIR @@ -1391,7 +1428,7 @@ void x11_block(void) { errExit("strdup"); profile_check_line(cmd, 0, NULL); profile_add(cmd); - char *xauthority = getenv("XAUTHORITY"); + const char *xauthority = env_get("XAUTHORITY"); if (xauthority) { char *line; if (asprintf(&line, "blacklist %s", xauthority) == -1) -- cgit v1.2.3-70-g09d2