diff --git a/etc/firejail.config b/etc/firejail.config index d34bb97be..330cc3b54 100644 --- a/etc/firejail.config +++ b/etc/firejail.config @@ -48,6 +48,18 @@ # cannot be overridden by --noblacklist or --ignore. # disable-mnt no +# Maximum number of environment variables. +# This limit is intended to make stack smashing harder (see +# https://github.com/netblue30/firejail/issues/4633). +# env-max-count 256 + +# Maximum length for each environment variable value. +# Example: `FOO=barr` has a length of 4. +# This limit is intended to make stack smashing harder (see +# https://github.com/netblue30/firejail/issues/4633). +# Note: The actual default value is based on `PATH_MAX`; see checkcfg.c. +# env-max-len 4096 + # Enable or disable file transfer support, default enabled. # file-transfer yes diff --git a/src/firejail/checkcfg.c b/src/firejail/checkcfg.c index 35030cf99..f270d1a9a 100644 --- a/src/firejail/checkcfg.c +++ b/src/firejail/checkcfg.c @@ -36,6 +36,8 @@ char *xvfb_extra_params = ""; char *netfilter_default = NULL; int arg_max_count = 128; // maximum number of command arguments (argc) unsigned long arg_max_len = 4096; // --foobar=PATH +int env_max_count = 256; // some sane maximum number of environment variables +unsigned long env_max_len = (PATH_MAX + 32); // FOOBAR=SOME_PATH, only applied to Firejail's own sandboxed apps unsigned long join_timeout = 5000000; // microseconds char *config_seccomp_error_action_str = "EPERM"; char *config_seccomp_filter_add = NULL; @@ -239,6 +241,25 @@ int checkcfg(int val) { else if (strncmp(ptr, "arg-max-len ", 12) == 0) arg_max_len = strtoul(ptr + 12, NULL, 10); + // env max count + else if (strncmp(ptr, "env-max-count ", 14) == 0) { + long tmp = strtol(ptr + 14, NULL, 10); + if (tmp < 0 || tmp >= INT_MAX) { + if (arg_debug) { + printf("env-max-count out of range: %ld, using %d\n", + tmp, INT_MAX); + } + env_max_count = INT_MAX; + } + else { + env_max_count = (int)tmp; + } + } + + // env max len + else if (strncmp(ptr, "env-max-len ", 12) == 0) + env_max_len = strtoul(ptr + 12, NULL, 10); + // arp probes else if (strncmp(ptr, "arp-probes ", 11) == 0) { int arp_probes = atoi(ptr + 11); diff --git a/src/firejail/env.c b/src/firejail/env.c index 4489ad0c2..f1e1f5d41 100644 --- a/src/firejail/env.c +++ b/src/firejail/env.c @@ -283,8 +283,9 @@ static void env_apply_list(const char * const *list, unsigned int num_items) { for (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); + if (strlen(env->name) + strlen(env->value) >= env_max_len) { + fprintf(stderr, "Error: too long environment variable value: '%s' len (%zu) >= env-max-len (%lu): '%s'; see --rmenv\n", + env->name, strlen(env->value), env_max_len, env->value); exit(1); } diff --git a/src/firejail/firejail.h b/src/firejail/firejail.h index e4c155cd4..0f869df91 100644 --- a/src/firejail/firejail.h +++ b/src/firejail/firejail.h @@ -739,8 +739,6 @@ int check_namespace_virt(void); 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, only applied to Firejail's own sandboxed apps // env.c typedef enum { SETENV = 0, @@ -879,6 +877,8 @@ extern char *xvfb_extra_params; extern char *netfilter_default; extern int arg_max_count; extern unsigned long arg_max_len; +extern int env_max_count; +extern unsigned long env_max_len; extern unsigned long join_timeout; extern char *config_seccomp_error_action_str; extern char *config_seccomp_filter_add; diff --git a/src/firejail/main.c b/src/firejail/main.c index 56ac19c82..ad022d4d9 100644 --- a/src/firejail/main.c +++ b/src/firejail/main.c @@ -1075,7 +1075,7 @@ int main(int argc, char **argv, char **envp) { // check standard streams before opening any file fix_std_streams(); - // initialize values from firejail.config (needed for arg_max_count) + // initialize values from firejail.config (needed for arg/env checks) checkcfg(0); // argument count should be larger than 0 @@ -1098,12 +1098,13 @@ int main(int argc, char **argv, char **envp) { } // Stash environment variables - for (i = 0, ptr = envp; ptr && *ptr && i < MAX_ENVS; i++, ptr++) + for (i = 0, ptr = envp; ptr && *ptr && i < env_max_count; i++, ptr++) env_store(*ptr, SETENV); // sanity check for environment variables - if (i >= MAX_ENVS) { - fprintf(stderr, "Error: too many environment variables: >= MAX_ENVS (%d)\n", MAX_ENVS); + if (i >= env_max_count) { + fprintf(stderr, "Error: too many environment variables: >= env-max-count (%d)\n", + env_max_count); exit(1); }