feature: add arg-max-count and arg-max-len options to firejail.config (#6878)

Replace the hardcoded `MAX_ARGS` and `MAX_ARG_LEN` limits with new
global configuration options, `arg-max-count` and `arg-max-len`, which
limit the maximum number of command-line arguments and the maximum
length of each argument (respectively).

Closes #4633.

Co-authored-by: Kelvin M. Klann <kmk3.code@protonmail.com>

etc/firejail.config

@@ -9,6 +9,18 @@
 # Enable AppArmor functionality, default enabled.
 # apparmor yes
 
+# Maximum number of arguments in the command line.
+# Example: `firejail --foo /usr/bin/bar baz` has 4 arguments.
+# This limit is intended to make stack smashing harder (see
+# https://github.com/netblue30/firejail/issues/4633).
+# arg-max-count 128
+
+# Maximum length of each argument in the command line.
+# Example: `--foo=bar` has a length of 9.
+# This limit is intended to make stack smashing harder (see
+# https://github.com/netblue30/firejail/issues/4633).
+# arg-max-len 4096
+
 # Number of ARP probes sent when assigning an IP address for --net option,
 # default 2. This is a partial implementation of RFC 5227. A 0.5 seconds
 # timeout is implemented for each probe. Increase this number to 4 if your

src/firejail/checkcfg.c

@@ -22,6 +22,7 @@
 #include "../include/syscall.h"
 #include <sys/stat.h>
 #include <linux/loop.h>
+#include <limits.h>
 
 #define MAX_READ 8192				  // line buffer for profile files
 
@@ -33,6 +34,8 @@ char *xpra_extra_params = "";
 char *xvfb_screen = "800x600x24";
 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
 unsigned long join_timeout = 5000000; // microseconds
 char *config_seccomp_error_action_str = "EPERM";
 char *config_seccomp_filter_add = NULL;
@@ -216,6 +219,26 @@ int checkcfg(int val) {
 				else
 					goto errout;
 			}
+
+			// arg max count
+			else if (strncmp(ptr, "arg-max-count ", 14) == 0) {
+				long tmp = strtol(ptr + 14, NULL, 10);
+				if (tmp < 0 || tmp >= INT_MAX) {
+					if (arg_debug) {
+						printf("arg-max-count out of range: %ld, using %d\n",
+						       tmp, INT_MAX);
+					}
+					arg_max_count = INT_MAX;
+				}
+				else {
+					arg_max_count = (int)tmp;
+				}
+			}
+
+			// arg max len
+			else if (strncmp(ptr, "arg-max-len ", 12) == 0)
+				arg_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/firejail.h

@@ -397,9 +397,7 @@ extern pid_t sandbox_pid;
 extern mode_t orig_umask;
 extern unsigned long long start_timestamp;
 
-#define MAX_ARGS 128		// maximum number of command arguments (argc)
-#define MAX_ARG_LEN (PATH_MAX + 32) // --foobar=PATH
-extern char *fullargv[MAX_ARGS];
+extern char **fullargv;
 extern int fullargc;
 
 // main.c
@@ -879,6 +877,8 @@ extern char *xpra_extra_params;
 extern char *xvfb_screen;
 extern char *xvfb_extra_params;
 extern char *netfilter_default;
+extern int arg_max_count;
+extern unsigned long arg_max_len;
 extern unsigned long join_timeout;
 extern char *config_seccomp_error_action_str;
 extern char *config_seccomp_filter_add;

src/firejail/main.c

@@ -176,7 +176,7 @@ int arg_restrict_namespaces = 0;
 int parent_to_child_fds[2];
 int child_to_parent_fds[2];
 
-char *fullargv[MAX_ARGS];			// expanded argv for restricted shell
+char **fullargv = NULL;				// expanded argv for restricted shell
 int fullargc = 0;
 static pid_t child = 0;
 pid_t sandbox_pid;
@@ -1075,20 +1075,24 @@ 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)
+	checkcfg(0);
+
 	// argument count should be larger than 0
 	if (argc == 0 || !argv || strlen(argv[0]) == 0) {
 		fprintf(stderr, "Error: argv is invalid\n");
 		exit(1);
-	} else if (argc >= MAX_ARGS) {
-		fprintf(stderr, "Error: too many arguments: argc (%d) >= MAX_ARGS (%d)\n", argc, MAX_ARGS);
+	} else if (argc >= arg_max_count) {
+		fprintf(stderr, "Error: too many arguments: argc (%d) >= arg-max-count (%d)\n",
+		        argc, arg_max_count);
 		exit(1);
 	}
 
 	// sanity check for arguments
 	for (i = 0; i < argc; i++) {
-		if (strlen(argv[i]) >= MAX_ARG_LEN) {
-			fprintf(stderr, "Error: too long argument: argv[%d] len (%zu) >= MAX_ARG_LEN (%d): %s\n",
-			        i, strlen(argv[i]), MAX_ARG_LEN, argv[i]);
+		if (strlen(argv[i]) >= arg_max_len) {
+			fprintf(stderr, "Error: too long argument: argv[%d] len (%zu) >= arg-max-len (%lu): '%s'\n",
+			        i, strlen(argv[i]), arg_max_len, argv[i]);
 			exit(1);
 		}
 	}
@@ -1247,9 +1251,28 @@ int main(int argc, char **argv, char **envp) {
 	}
 	EUID_ASSERT();
 
+#ifndef ARGC_MAX_RESTRICTED_SHELL
+#define ARGC_MAX_RESTRICTED_SHELL 4096
+#endif
 	// is this a login shell, or a command passed by sshd,
 	// insert command line options from /etc/firejail/login.users
 	if (*argv[0] == '-' || parent_sshd) {
+		// use a sane size for allocation
+		int fullargv_sz = arg_max_count;
+		if (fullargv_sz > ARGC_MAX_RESTRICTED_SHELL) {
+			if (arg_debug) {
+				printf("arg-max-count %d > %d, allocating %d elements for fullargv\n",
+				       arg_max_count, ARGC_MAX_RESTRICTED_SHELL,
+				       ARGC_MAX_RESTRICTED_SHELL);
+			}
+			fullargv_sz = ARGC_MAX_RESTRICTED_SHELL;
+		}
+
+		fullargv = malloc(fullargv_sz * sizeof(char *));
+		if (!fullargv)
+			errExit("malloc");
+		memset(fullargv, 0, fullargv_sz * sizeof(char *));
+
 		if (argc == 1)
 			login_shell = 1;
 		fullargc = restricted_shell(cfg.username);
@@ -1270,7 +1293,7 @@ int main(int argc, char **argv, char **envp) {
 #endif
 
 			int j;
-			for (i = 1, j = fullargc; i < argc && j < MAX_ARGS; i++, j++, fullargc++)
+			for (i = 1, j = fullargc; i < argc && j < fullargv_sz; i++, j++, fullargc++)
 				fullargv[j] = argv[i];
 
 			// replace argc/argv with fullargc/fullargv

src/firejail/restricted_shell.c

@@ -86,10 +86,11 @@ int restricted_shell(const char *user) {
 		if (fnmatch(usr, user, 0) == 0) {
 			// process program arguments
 
+			assert(fullargv != NULL);
 			fullargv[0] = "firejail";
 			int i;
 			ptr = args;
-			for (i = 1; i < MAX_ARGS; i++) {
+			for (i = 1; i < arg_max_count; i++) {
 				// skip blanks
 				while (*ptr == ' ' || *ptr == '\t')
 					ptr++;