Commit Diff

Commit:
5395dccaaf6468b5e829470cd91fb8e60e89766e
From:
Martijn van Duren <martijn@openbsd.org>
Date:
Message:
Put shell command building inside child process so it can't take down the server
Actions:
Patch | Tree
commit - 3f06523c090bbbb5d7651f30f3c2c498ae55d9ce
commit + 5395dccaaf6468b5e829470cd91fb8e60e89766e
blob - 805282e7572602555459334e2b0882dd5de79669
blob + 4a3d57fa52d2dd981fdb1deb28c23527489d0ace
--- main.c
+++ main.c
@@ -204,33 +204,6 @@ extends_exec(struct imsg *imsg)
 	if (cmd->pid != -1)
 		fatal("%s: command already running", cmd->token);
 
-	switch (cmd->exectype) {
-	case exectype_exec:
-		file = cmd->command;
-		for (i = 0; cmd->args != NULL && cmd->args[i] != NULL; i++)
-			;
-		if ((argv = calloc(i + 2, sizeof(*argv))) == NULL)
-			fatal(NULL);
-		argv[0] = cmd->command;
-		for (i = 0; cmd->args != NULL && cmd->args[i] != NULL; i++)
-			argv[i + 1] = cmd->args[i];
-		argv[i + 1] = NULL;
-		break;
-	case exectype_shell:
-		strlcpy(shcmd, cmd->command, sizeof(shcmd));
-		for (i = 0; cmd->args != NULL && cmd->args[i] != NULL; i++) {
-			if (strlcat(shcmd, " ", sizeof(shcmd)) >= sizeof(shcmd))
-				fatal("%s: command too long", cmd->token);
-			if (strlcat(shcmd, cmd->args[i], sizeof(shcmd)) >=
-			    sizeof(shcmd))
-				fatal("%s: command too long", cmd->token);
-		}
-		argv = shargv;
-		break;
-	default:
-		fatal("%s: unexpected exectype: %d", __func__, cmd->exectype);
-	}
-
 	if (pipe(in) == -1 || pipe(out) == -1 || pipe(err) == -1)
 		fatal("%s: pipe", __func__);
 	switch (cmd->pid = fork()) {
@@ -254,6 +227,34 @@ extends_exec(struct imsg *imsg)
 			fatal("setgid(%u)", cmd->group);
 		if (setuid(cmd->user) == -1)
 			fatal("setuid(%u)", cmd->user);
+
+		switch (cmd->exectype) {
+		case exectype_exec:
+			file = cmd->command;
+			for (i = 0; cmd->args != NULL && cmd->args[i] != NULL; i++)
+				;
+			if ((argv = calloc(i + 2, sizeof(*argv))) == NULL)
+				fatal(NULL);
+			argv[0] = cmd->command;
+			for (i = 0; cmd->args != NULL && cmd->args[i] != NULL; i++)
+				argv[i + 1] = cmd->args[i];
+			argv[i + 1] = NULL;
+			break;
+		case exectype_shell:
+			strlcpy(shcmd, cmd->command, sizeof(shcmd));
+			for (i = 0; cmd->args != NULL && cmd->args[i] != NULL; i++) {
+				if (strlcat(shcmd, " ", sizeof(shcmd)) >= sizeof(shcmd))
+					fatal("%s: command too long", cmd->token);
+				if (strlcat(shcmd, cmd->args[i], sizeof(shcmd)) >=
+				    sizeof(shcmd))
+					fatal("%s: command too long", cmd->token);
+			}
+			argv = shargv;
+			break;
+		default:
+			fatal("%s: unexpected exectype: %d", __func__, cmd->exectype);
+		}
+
 		execvpe(file, argv, cmd->env);
 		log_warn("execve");
 		_exit(127);
blob - 4de49cb60c90618aff96a3404061a317553527cc
blob + 320c412c84846537ddcc069653be9c678513ca40
--- parse.y
+++ parse.y
@@ -339,7 +339,7 @@ command		: STRING '{' optnl {
 
 			command = RB_INSERT(commands, &conf->commands, command);
 			if (command != NULL) {
-				yyerror("Duplicate extend: %s", extend->token);
+				yyerror("Duplicate extend: %s", command->token);
 				YYERROR;
 			}
 			command = NULL;