feature: add env-max-count / env-max-len to firejail.config (#6951)

Replace the hardcoded `MAX_ENVS` and `MAX_ENV_LEN` limits with new
global configuration options, `env-max-count` and `env-max-len`, which
limit the maximum number of environment variables and the maximum length
of each environment variable (respectively).

Also, include the environment name and value in the "too long
environment variable" error message, similarly to the "too long
argument" error message (see PR #4676 and PR #5677).

This is a follow-up to #6878.

Closes #3678.

Author: kmk3

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
 

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);

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);
 					}
 

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;

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);
 	}