Prevent extra arguments unless explicitly allowed

src/linker.c

@@ -20,6 +20,7 @@ static struct argp_option options[] = {
 	{"output", 'o', "FILE", 0, "Path to output binary"},
 	{"force", 'f', 0, 0, "Overwrite output file if it already exists"},
 	{"quiet", 'q', 0, 0, "Suppress all output"},
+	{"allow-extra-args", 'e', 0, 0, "Allow extra arguments to be passed to the generated binary"},
 	{0}
 };
 
@@ -37,6 +38,7 @@ struct arguments
 	int argc;
 	char **argv;
 	bool force;
+	bool allow_extra_args;
 };
 
 static error_t parse_opt(int key, char *arg, struct argp_state *state)
@@ -62,6 +64,10 @@ static error_t parse_opt(int key, char *arg, struct argp_state *state)
 		arguments->force = true;
 		break;
 
+	case 'e':
+		arguments->allow_extra_args = true;
+		break;
+
 	case 'q':
 		log_info = log_silent;
 		log_error = log_silent;
@@ -89,7 +95,8 @@ static struct argp argp = {options, parse_opt, args_doc, doc};
 
 const uint64_t BUFFER_SIZE = 1024 * 1024;
 
-int generate_binary(struct arguments arguments);
+static int generate_binary(struct arguments arguments);
+static wrapper *wrapper_build_from_args(struct arguments arguments);
 
 extern char _binary_runner_start[];
 extern char _binary_runner_end[];
@@ -98,6 +105,7 @@ int main(int argc, char **argv)
 {
 	struct arguments arguments;
 	arguments.force = false;
+	arguments.allow_extra_args = false;
 	arguments.mode = link;
 	arguments.inspect = NULL;
 	arguments.output = "a.out";
@@ -133,15 +141,16 @@ int main(int argc, char **argv)
 		{
 			log_info("Argument %li: %s\n", i, wrapper->argv[i]);
 		}
+		log_info("Allow extra arguments: %s\n", wrapper->allow_extra_args ? "yes" : "no");
 		wrapper_destroy(wrapper);
 
-		return 1;
+		return 0;
 	}
 
 	return generate_binary(arguments);
 }
 
-int generate_binary(struct arguments arguments)
+static int generate_binary(struct arguments arguments)
 {
 	FILE *output = NULL;
 	if (arguments.force)
@@ -176,7 +185,7 @@ int generate_binary(struct arguments arguments)
 		return 1;
 	}
 
-	wrapper *wrapper = wrapper_build_from_args(arguments.argc, arguments.argv);
+	wrapper *wrapper = wrapper_build_from_args(arguments);
 	if (wrapper == NULL)
 	{
 		return 1;
@@ -197,6 +206,7 @@ int generate_binary(struct arguments arguments)
 		fwrite(&len, sizeof(len), 1, output);
 		fwrite(wrapper->argv[i], 1, len, output);
 	}
+	fwrite(&wrapper->allow_extra_args, sizeof(wrapper->allow_extra_args), 1, output);
 	wrapper_destroy(wrapper);
 
 	fwrite(&runner_size, sizeof(runner_size), 1, output);
@@ -206,3 +216,32 @@ int generate_binary(struct arguments arguments)
 
 	return 0;
 }
+
+static wrapper *wrapper_build_from_args(struct arguments arguments)
+{
+	wrapper *result = wrapper_new(arguments.argc);
+	if (result == NULL)
+	{
+		return NULL;
+	}
+
+	for (int i = 0; i < result->argc; i++)
+	{
+		argv_len_t len = strlen(arguments.argv[i]);
+		result->argv[i] = (char *)malloc(sizeof(char) * (len + 1));
+		if (result->argv[i] == NULL)
+		{
+			log_error("Failed to allocate wrapper argv[%i] with length %i!\n", i, len);
+			wrapper_destroy(result);
+			return NULL;
+		}
+		strcpy(result->argv[i], arguments.argv[i]);
+	}
+
+	if (arguments.allow_extra_args)
+	{
+		result->allow_extra_args = true;
+	}
+
+	return result;
+}

src/runner.c

@@ -21,11 +21,18 @@ int main(int argc, char **argv)
 		new_argv[i] = wrapper->argv[i];
 	}
 
-	for (int i = 0; i < argc - 1; i++)
+	if (wrapper->allow_extra_args)
 	{
-		new_argv[wrapper->argc + i] = argv[i + 1];
+		for (int i = 0; i < argc - 1; i++)
+		{
+			new_argv[wrapper->argc + i] = argv[i + 1];
+		}
+		new_argv[wrapper->argc + argc - 1] = NULL;
+	}
+	else
+	{
+		new_argv[wrapper->argc] = NULL;
 	}
-	new_argv[wrapper->argc + argc - 1] = NULL;
 
 	if (setuid(geteuid()) != 0)
 	{

src/wrapper.c

@@ -54,30 +54,7 @@ wrapper *wrapper_new(int argc)
 		return NULL;
 	}
 	memset(result->argv, 0, argv_size);
-
-	return result;
-}
-
-wrapper *wrapper_build_from_args(int argc, char **argv)
-{
-	wrapper *result = wrapper_new(argc);
-	if (result == NULL)
-	{
-		return NULL;
-	}
-
-	for (int i = 0; i < result->argc; i++)
-	{
-		argv_len_t len = strlen(argv[i]);
-		result->argv[i] = (char *)malloc(sizeof(char) * (len + 1));
-		if (result->argv[i] == NULL)
-		{
-			log_error("Failed to allocate wrapper argv[%i] with length %i!\n", i, len);
-			wrapper_destroy(result);
-			return NULL;
-		}
-		strcpy(result->argv[i], argv[i]);
-	}
+	result->allow_extra_args = false;
 
 	return result;
 }
@@ -127,6 +104,7 @@ wrapper *wrapper_build_from_runner(FILE *runner_exe)
 		log_error("Executable was not created with " APP_NAME ", or was created with an incompatible version\n");
 		return NULL;
 	}
+	log_debug("Marker check passed\n");
 
 	version version;
 	read_or_return(&version, runner_exe, NULL, "Failed to read runner version\n");
@@ -138,6 +116,7 @@ wrapper *wrapper_build_from_runner(FILE *runner_exe)
 		log_error("Runner was created with an incompatible version of " APP_NAME "\n");
 		return NULL;
 	}
+	log_debug("Version check passed\n");
 
 	argc_t argc;
 	read_or_return(&argc, runner_exe, NULL, "Failed to read argument count\n");
@@ -167,6 +146,8 @@ wrapper *wrapper_build_from_runner(FILE *runner_exe)
 		result->argv[i][len] = 0;
 	}
 
+	read_or_return(&result->allow_extra_args, runner_exe, NULL, "Failed to read allow_extra_args\n");
+
 	return result;
 }
 

src/wrapper.h

@@ -3,6 +3,7 @@
 
 #include <stdint.h>
 #include <stdio.h>
+#include <stdbool.h>
 
 #ifndef APP_NAME
 	#define APP_NAME "suid-wrapper"
@@ -35,6 +36,7 @@ typedef struct wrapper
 {
 	argc_t argc;
 	char **argv;
+	bool allow_extra_args;
 } wrapper;
 
 typedef struct version
@@ -46,7 +48,7 @@ typedef struct version
 
 version get_version();
 
-wrapper *wrapper_build_from_args(argc_t argc, char **argv);
+wrapper *wrapper_new(int argc);
 wrapper *wrapper_build_from_runner(FILE *runner_exe);
 void wrapper_destroy(wrapper *wrapper);