Skip to content

Commit

Permalink
Do not use "/bin/sh" to run external commands.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
npat-efault committed Aug 18, 2015
1 parent d068755 commit 1ebc60b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 32 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ CPPFLAGS += -DSEND_RECEIVE_HISTFILE=\"$(SEND_RECEIVE_HISTFILE)\" \
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
Expand Down
63 changes: 33 additions & 30 deletions picocom.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#define _GNU_SOURCE
#include <getopt.h>

#include "split.h"
#include "term.h"
#ifdef LINENOISE
#include "linenoise-1.0/linenoise.h"
Expand Down Expand Up @@ -721,6 +722,9 @@ show_status (int dtr_up)

/**********************************************************************/

#define RUNCMD_ARGS_MAX 32
#define RUNCMD_EXEC_FAIL 126

void
establish_child_signal_handlers (void)
{
Expand All @@ -735,10 +739,8 @@ establish_child_signal_handlers (void)
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;
Expand Down Expand Up @@ -781,8 +783,10 @@ run_cmd(int fd, ...)
} 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 */
Expand All @@ -796,37 +800,36 @@ run_cmd(int fd, ...)
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
Expand Down Expand Up @@ -944,7 +947,7 @@ do_command (unsigned char c)
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:
Expand Down

0 comments on commit 1ebc60b

Please sign in to comment.