CVE-2015-9059: Improper Neutralization of Special Elements used in a Command ('Command Injection') (CWE-77)
value: picocom before 2.0 has a command injection vulnerability in the 'send and receive file' command because the command line is executed by /bin/sh unsafely.
CWE description: The product constructs all or part of a command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended command when it is sent to a downstream component.
CVSS v3 base score: 9.8
Published: 2017-05-28T00:29Z
Repository: https://github.com/npat-efault/picocom
Language: Other
Files changed: Makefile, picocom.c
Commit date: 2015-08-18 14:40:48 +0300
Commit message: Do not use "/bin/sh" to run external commands.

Picocom no longer uses /bin/sh to run external commands for
file-transfer operations. Parsing the command line and spliting it into
arguments is now performed internally by picocom, using quoting rules
very similar to those of the Unix shell. Hopefully, this makes it
impossible to inject shell-commands when supplying filenames or
extra arguments to the send- and receive-file commands.

Diff: diff --git a/Makefile b/Makefile
index ed37bfd..b110bd5 100644
--- a/Makefile
+++ b/Makefile
@@ -31,16 +31,16 @@ SEND_RECEIVE_HISTFILE = .picocom_send_receive
 CPPFLAGS += -DSEND_RECEIVE_HISTFILE=\"$(SEND_RECEIVE_HISTFILE)\" \
 	    -DLINENOISE
 picocom : linenoise-1.0/linenoise.o
 linenoise-1.0/linenoise.o : linenoise-1.0/linenoise.c linenoise-1.0/linenoise.h
 
-
-picocom : picocom.o term.o
+picocom : picocom.o term.o split.o
 #	$(LD) $(LDFLAGS) -o $@ $+ $(LDLIBS)
 
 picocom.o : picocom.c term.h
 term.o : term.c term.h
+split.o : split.c split.h
 
 
 doc : picocom.8 picocom.8.html picocom.8.ps
 
 changes :
diff --git a/picocom.c b/picocom.c
index 432e3b7..e4e898c 100644
--- a/picocom.c
+++ b/picocom.c
@@ -46,10 +46,11 @@
 #endif
 
 #define _GNU_SOURCE
 #include <getopt.h>
 
+#include "split.h"
 #include "term.h"
 #ifdef LINENOISE
 #include "linenoise-1.0/linenoise.h"
 #endif
 
@@ -719,10 +720,13 @@ show_status (int dtr_up)
 	fd_printf(STO, "*** dtr: %s\r\n", dtr_up ? "up" : "down");
 }
 
 /**********************************************************************/
 
+#define RUNCMD_ARGS_MAX 32
+#define RUNCMD_EXEC_FAIL 126
+
 void
 establish_child_signal_handlers (void)
 {
 	struct sigaction dfl_action;
 
@@ -733,14 +737,12 @@ establish_child_signal_handlers (void)
 	
 	sigaction (SIGINT, &dfl_action, NULL);
 	sigaction (SIGTERM, &dfl_action, NULL);
 }
 
-#define EXEC "exec "
-
 int
-run_cmd(int fd, ...)
+run_cmd(int fd, const char *cmd, const char *args_extra)
 {
 	pid_t pid;
 	sigset_t sigm, sigm_old;
 
 	/* block signals, let child establish its own handlers */
@@ -779,12 +781,14 @@ run_cmd(int fd, ...)
 			return -1;
 		}
 	} else {
 		/* child: external program */
 		long fl;
-		char cmd[512];
-
+		int argc;
+		char *argv[RUNCMD_ARGS_MAX + 1];
+		int r;
+			
 		/* unmanage terminal, and reset it to canonical mode */
 		term_remove(STI);
 		/* unmanage serial port fd, without reset */
 		term_erase(fd);
 		/* set serial port fd to blocking mode */
@@ -794,41 +798,40 @@ run_cmd(int fd, ...)
 		/* connect stdin and stdout to serial port */
 		close(STI);
 		close(STO);
 		dup2(fd, STI);
 		dup2(fd, STO);
-		{
-			/* build command-line */
-			char *c, *ce;
-			const char *s;
-			int n;
-			va_list vls;
-			
-			strcpy(cmd, EXEC);
-			c = &cmd[sizeof(EXEC)- 1];
-			ce = cmd + sizeof(cmd) - 1;
-			va_start(vls, fd);
-			while ( (s = va_arg(vls, const char *)) ) {
-				n = strlen(s);
-				if ( c + n + 1 >= ce ) break;
-				memcpy(c, s, n); c += n;
-				*c++ = ' ';
-			}
-			va_end(vls);
-			*c = '\0';
+		
+		/* build command arguments vector */
+		argc = 0;
+		r = split_quoted(cmd, &argc, argv, RUNCMD_ARGS_MAX);
+		if ( r < 0 ) {
+			fd_printf(STDERR_FILENO, "Cannot parse command\n");
+			exit(RUNCMD_EXEC_FAIL);
+		}
+		r = split_quoted(args_extra, &argc, argv, RUNCMD_ARGS_MAX);
+		if ( r < 0 ) {
+			fd_printf(STDERR_FILENO, "Cannot parse extra args\n");
+			exit(RUNCMD_EXEC_FAIL);
 		}
+		if ( argc < 1 ) {
+			fd_printf(STDERR_FILENO, "No command given\n");
+			exit(RUNCMD_EXEC_FAIL);
+		}	
+		argv[argc] = NULL;
+			
 		/* run extenral command */
-		fd_printf(STDERR_FILENO, "%s\n", &cmd[sizeof(EXEC) - 1]);
+		fd_printf(STDERR_FILENO, "$ %s %s\n", cmd, args_extra);
 		establish_child_signal_handlers();
 		sigprocmask(SIG_SETMASK, &sigm_old, NULL);
-		execl("/bin/sh", "sh", "-c", cmd, NULL);
-		exit(42);
+		execvp(argv[0], argv);
+
+		fd_printf(STDERR_FILENO, "exec: %s\n", strerror(errno));
+		exit(RUNCMD_EXEC_FAIL);
 	}
 }
 
-#undef EXEC
-
 /**********************************************************************/
 
 /* Process command key. Returns non-zero if command results in picocom
    exit, zero otherwise. */
 int
@@ -942,11 +945,11 @@ do_command (unsigned char c)
 		fname = read_filename();
 		if (fname == NULL) {
 			fd_printf(STO, "*** cannot read filename ***\r\n");
 			break;
 		}
-		run_cmd(tty_fd, xfr_cmd, fname, NULL);
+		run_cmd(tty_fd, xfr_cmd, fname);
 		free(fname);
 		break;
 	case KEY_BREAK:
 		term_break(tty_fd);
 		fd_printf(STO, "\r\n*** break sent ***\r\n");

Vulnerable code: 
picocom : picocom.o term.o
#define EXEC "exec "

run_cmd(int fd, ...)
		char cmd[512];

		{
			/* build command-line */
			char *c, *ce;
			const char *s;
			int n;
			va_list vls;
			
			strcpy(cmd, EXEC);
			c = &cmd[sizeof(EXEC)- 1];
			ce = cmd + sizeof(cmd) - 1;
			va_start(vls, fd);
			while ( (s = va_arg(vls, const char *)) ) {
				n = strlen(s);
				if ( c + n + 1 >= ce ) break;
				memcpy(c, s, n); c += n;
				*c++ = ' ';
			}
			va_end(vls);
			*c = '\0';
		fd_printf(STDERR_FILENO, "%s\n", &cmd[sizeof(EXEC) - 1]);
		execl("/bin/sh", "sh", "-c", cmd, NULL);
		exit(42);
#undef EXEC

		run_cmd(tty_fd, xfr_cmd, fname, NULL);
Fixed code: picocom : picocom.o term.o split.o
split.o : split.c split.h
#include "split.h"
#define RUNCMD_ARGS_MAX 32
#define RUNCMD_EXEC_FAIL 126

run_cmd(int fd, const char *cmd, const char *args_extra)
		int argc;
		char *argv[RUNCMD_ARGS_MAX + 1];
		int r;
			
		
		/* build command arguments vector */
		argc = 0;
		r = split_quoted(cmd, &argc, argv, RUNCMD_ARGS_MAX);
		if ( r < 0 ) {
			fd_printf(STDERR_FILENO, "Cannot parse command\n");
			exit(RUNCMD_EXEC_FAIL);
		}
		r = split_quoted(args_extra, &argc, argv, RUNCMD_ARGS_MAX);
		if ( r < 0 ) {
			fd_printf(STDERR_FILENO, "Cannot parse extra args\n");
			exit(RUNCMD_EXEC_FAIL);
		if ( argc < 1 ) {
			fd_printf(STDERR_FILENO, "No command given\n");
			exit(RUNCMD_EXEC_FAIL);
		}	
		argv[argc] = NULL;
			
		fd_printf(STDERR_FILENO, "$ %s %s\n", cmd, args_extra);
		execvp(argv[0], argv);

		fd_printf(STDERR_FILENO, "exec: %s\n", strerror(errno));
		exit(RUNCMD_EXEC_FAIL);
		run_cmd(tty_fd, xfr_cmd, fname);