Add timeout to run_command_write_fd and replace popen by execvpe

dump_systemstate runs many external commands. When one hungs,
dump_systemstate will block the crash-worker.

execvpe replaced popen because popen passes command to /bin/sh, and this
can potentially allow to run the malicious command if attacker can
modify the fragment of cmd, e.g:

  void read_file(char *filename)
  {
      ...
      sprintf(buff, "cat %s", filename);
      popen(buff, "r");
      ...
  }

  main()
  {
      char filename[] = "/etc/passwd;rm -rf /";
      read_file(filename);
  }

Change-Id: Id7b37c058869c27d3c4d282d9d2dd30d5b9ec80c

diff --git a/src/dump_systemstate/dump_systemstate.c b/src/dump_systemstate/dump_systemstate.c
index 0bf881e..1c5296e 100644 (file)
--- a/src/dump_systemstate/dump_systemstate.c
+++ b/src/dump_systemstate/dump_systemstate.c
@@ -37,6 +37,9 @@
 #include "shared/log.h"
 
 #define FILE_PERM (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH)
+
+#define TIMEOUT_DEFAULT 60
+
 static struct dump_item {
        const char *title;
        const char *path;
@@ -147,69 +150,83 @@ int main(int argc, char *argv[])
        fprintf_fd(out_fd, "\n");
 
        fprintf_fd(out_fd, "\n==== System disk space usage (/bin/df -h)\n");
-       ret = run_command_write_fd("/bin/df -h", out_fd);
+       char *df_args[] = {"/bin/df", "-h", NULL};
+       ret = run_command_write_fd_timeout(df_args[0], df_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
        if (ret < 0)
                goto exit_close;
 
        dpercent = get_disk_used_percent("/opt");
        if (90 < dpercent) {
                fprintf_fd(out_fd, "\n==== System disk space usage detail - %d\% (/bin/du -ah /opt)\n", dpercent);
-               ret = run_command_write_fd("/usr/bin/du -ah /opt --exclude=/opt/usr", out_fd);
+               char *du_args[] = {"/bin/du", "-ah", "/opt", "--exclude=/opt/usr", NULL};
+               ret = run_command_write_fd_timeout(du_args[0], du_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
        }
        fprintf_fd(out_fd, "\n==== System timezone (ls -al /opt/etc/localtime)\n");
-       ret = run_command_write_fd("ls -al /opt/etc/localtime", out_fd);
+       char *ls_args[] = {"/bin/ls", "-al", "/opt/etc/localtime", NULL};
+       ret = run_command_write_fd_timeout(ls_args[0], ls_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
        if (ret < 0)
                goto exit_close;
 
        fprintf_fd(out_fd, "\n==== System summary (/usr/bin/top -bcH -n 1)\n");
-       ret = run_command_write_fd("COLUMNS=200 /usr/bin/top -bcH -n 1", out_fd);
+       char *top_args[] = {"/bin/top", "-bcH", "-n", "1", NULL};
+       char *top_env[] = {"COLUMNS=200", NULL};
+       ret = run_command_write_fd_timeout(top_args[0], top_args, top_env, out_fd, NULL, 0, TIMEOUT_DEFAULT);
        if (ret < 0)
                goto exit_close;
 
        fprintf_fd(out_fd, "\n==== Current processes (/bin/ps auxfw)\n");
-       ret = run_command_write_fd("/bin/ps auxfw", out_fd);
+       char *ps_args[] = {"/bin/ps", "auxfw", NULL};
+       ret = run_command_write_fd_timeout(ps_args[0], ps_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
        if (ret < 0)
                goto exit_close;
 
        fprintf_fd(out_fd, "\n==== System memory statistics (/usr/bin/memps -v)\n");
-       ret = run_command_write_fd("/usr/bin/memps -v", out_fd);
+       char *memps_args[] = {"/bin/memps", "-v", NULL};
+       ret = run_command_write_fd_timeout(memps_args[0], memps_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
        if (ret < 0)
                goto exit_close;
 
        if (is_root) {
                fprintf_fd(out_fd, "\n==== System configuration (/usr/bin/vconftool get memory, db, file)\n");
-               ret = run_command_write_fd("/usr/bin/vconftool get memory/ -r", out_fd);
+               char *get_mem_args[] = {"/bin/vconftool", "get", "memory/", "-r", NULL};
+               ret = run_command_write_fd_timeout(get_mem_args[0], get_mem_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
 
-               ret = run_command_write_fd("/usr/bin/vconftool get db/ -r", out_fd);
+               char *get_db_args[] = {"/bin/vconftool", "get", "db/", "-r", NULL};
+               ret = run_command_write_fd_timeout(get_db_args[0], get_db_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
 
-               ret = run_command_write_fd("/usr/bin/vconftool get file/ -r", out_fd);
+               char *get_file_args[] = {"/bin/vconftool", "get", "file/", "-r", NULL};
+               ret = run_command_write_fd_timeout(get_file_args[0], get_file_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
        }
 
        if (arg_dmesg && is_root) {
                fprintf_fd(out_fd, "\n==== Kernel messages (TZ=UTC /bin/dmesg -T)\n");
-               ret = run_command_write_fd("TZ=UTC /bin/dmesg -T", out_fd);
+               char *dmesg_args[] = {"/bin/dmesg", "-T", NULL};
+               char *dmesg_env[] = {"TZ=UTC", NULL};
+               ret = run_command_write_fd_timeout(dmesg_args[0], dmesg_args, dmesg_env, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
        }
 
        if (arg_dlog) {
                fprintf_fd(out_fd, "\n==== Log messages\n");
-               ret = run_command_write_fd("/usr/bin/dlogutil -d -v threadtime -u 16384", out_fd);
+               char *dlogutil_args[] = {"/bin/dlogutil", "-d", "-v", "threadtime", "-u", "16384", NULL};
+               ret = run_command_write_fd_timeout(dlogutil_args[0], dlogutil_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
        }
 
        if (arg_journal) {
                fprintf_fd(out_fd, "\n==== Journal messages\n");
-               ret = run_command_write_fd("/usr/bin/journalctl -b -n 1024", out_fd);
+               char *journalctl_args[] = {"/bin/journalctl", "-b", "-n", "1024", NULL};
+               ret = run_command_write_fd_timeout(journalctl_args[0], journalctl_args, NULL, out_fd, NULL, 0, TIMEOUT_DEFAULT);
                if (ret < 0)
                        goto exit_close;
        }

diff --git a/src/shared/util.c b/src/shared/util.c
index e18e943..8075f12 100644 (file)
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -20,6 +20,7 @@
 #include <stdlib.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <sys/time.h>
 #include <dirent.h>
 #include <fcntl.h>
 #include <stdarg.h>
@@ -36,6 +37,9 @@
 #include "util.h"
 #include "log.h"
 
+#define READ_BUFF_SIZE 4096
+#define SELECT_TIMEOUT_US 100000
+
 int system_command_parallel(char *command)
 {
        int pid = 0;
@@ -285,27 +289,220 @@ int dump_file_write_fd(char *src, int dfd)
        return 1;
 }
 
-int run_command_write_fd(char *cmd, int dfd)
+static int run_command(char *path, char *args[], char *env[], int fd[])
 {
-       FILE *fp;
-       char buff[PIPE_BUF];
-       int ret;
+       if (dup2(fd[1], STDOUT_FILENO) == -1) {
+               _E("dup2 error: %m");
+               return -1;
+       }
 
-       if (!cmd) {
-               _E("Invalid argument\n");
+       if (close(fd[1]) == -1) {
+               _E("close fd error: %m");
                return -1;
        }
 
-       fp = popen(cmd, "r");
-       if (fp == NULL) {
-               _E("Failed to popen\n");
+       if (close(fd[0]) == -1) {
+               _E("close fd error: %m");
                return -1;
        }
-       while (fgets(buff, PIPE_BUF, fp) != NULL)
-               write_fd(dfd, buff, strlen(buff));
 
-       ret = pclose(fp);
-       return ret;
+       if (execvpe(path, args, env) == -1) {
+               _E("run command %s error: %m", path);
+               return -1;
+       }
+       return -1;
+}
+
+static void wait_for_child(pid_t pid, int timeout)
+{
+       for (int i = 0; i < 10*timeout; i++) {
+               int status;
+               pid_t p = waitpid(pid, &status, WNOHANG);
+               if (p == pid) {
+                       if (WIFSIGNALED(status)) {
+                               _I("Killed by signal %d\n", WTERMSIG(status));
+                               break;
+                       } else if (WIFEXITED(status)) {
+                               if (WEXITSTATUS(status) > 0)
+                                       _I("Exit code %d\n", WEXITSTATUS(status));
+                               break;
+                       }
+               } else if (p == -1) {
+                       _E("waitpid error: %m");
+                       break;
+               }
+               usleep(100000);
+       }
+}
+
+static int read_into_buff(int fd, char *buff, int size, int timeout_us, int *eof)
+{
+       struct timeval tout;
+       int sel_ret;
+       fd_set set;
+
+       FD_ZERO(&set);
+       FD_SET(fd, &set);
+
+       tout.tv_sec = timeout_us / 1000000;
+       tout.tv_usec = timeout_us % 1000000;
+       *eof = 0;
+
+       int buff_pos = 0;
+       if ((sel_ret = select(fd+1, &set, NULL, NULL, &tout)) >= 0) {
+               if (sel_ret > 0) {
+                       // we can do nonblocking read
+                       int readed = read(fd, &buff[buff_pos], size);
+
+                       if (readed > 0) {
+                               buff_pos += readed;
+                               size -= readed;
+                       } else if (readed == 0) {
+                               // no more data to read
+                               *eof = 1;
+                       } else {
+                               // error
+                               _E("read data from the pipe error: %m");
+                               return -1;
+                       }
+               }
+       } else
+               _E("select() error: %m");
+       return buff_pos;
+}
+
+// Usage:
+// if buff is not NULL then 'size' bytes of the result is written the buffer,
+// otherwise result is written to the dfd descriptor
+int run_command_write_fd_timeout(char *path, char *args[], char *env[], int dfd, char *buff, int size, int timeout)
+{
+       char BUFF[READ_BUFF_SIZE];
+       int fd[2];
+       struct timeval start, end;
+       int write_to_fd = buff == NULL ? 1 : 0;
+
+       if (!write_to_fd && size <= 0) {
+               _E("buffer size must be greather than zero");
+               return -1;
+       }
+
+       if (pipe(fd)) {
+               _E("pipe create error: %m");
+               return -1;
+       }
+
+       pid_t pid = fork();
+
+       if (pid == 0) {
+               return run_command(path, args, env, fd);
+       } else if (pid > 0) {
+               if (close(fd[1]) == -1) {
+                       _E("close fd error: %m");
+                       return -1;
+               }
+
+               if (gettimeofday(&start, NULL) == -1) {
+                       _E("gettimeofday error: %m");
+                       return -1;
+               }
+
+               int readed;
+               int eof = 0;
+               int outdated = 0;
+               int count = 0;
+
+               int act_size;
+               char *act_buff;
+
+               if (write_to_fd) {
+                       act_size = READ_BUFF_SIZE;
+                       act_buff = BUFF;
+               } else {
+                       act_size = size;
+                       act_buff = buff;
+               }
+
+               while ((readed = read_into_buff(fd[0], act_buff, act_size, 1000000, &eof)) >= 0) {
+                       if (readed > 0) {
+                               // we have some data
+                               if (count < (INT_MAX - readed))
+                                       count += readed;
+                               else
+                                       count = INT_MAX;
+
+                               if (write_to_fd) {
+                                       if (write_fd(dfd, act_buff, readed) == -1) {
+                                               _E("write data to pipe error: %m");
+                                               break;
+                                       }
+                               } else {
+                                       act_buff += readed;
+                                       act_size -= readed;
+
+                                       if (act_size == 0) {
+                                               // buff is full, we can return
+                                               eof = 1;
+                                       }
+                               }
+                       }
+
+                       if (eof)
+                               break;
+
+                       if (gettimeofday(&end, NULL) == -1) {
+                               _E("gettimeofday error: %m");
+                               break;
+                       }
+
+                       if ((end.tv_sec - start.tv_sec) > timeout) {
+                               outdated = 1;
+                               break;
+                       }
+
+                       if (readed == 0)
+                               usleep(100000);
+               }
+
+               if (outdated) {
+                       _E("command timeout: %s", path);
+                       if (kill(pid, 0) == 0) {
+                               // we can kill a child because we don't
+                               // need it anymore
+                               if (kill(pid, SIGTERM) == -1)
+                                       _E("kill child %d error: %m", pid);
+                       }
+               }
+
+               if (close(fd[0]) == -1) {
+                       _E("close fd error: %m");
+                       return -1;
+               }
+
+               // let's wait a second for a child
+               wait_for_child(pid, 1);
+
+               return eof == 1 ? count : -1;
+       } else {
+               _E("fork() error: %m");
+               return -1;
+       }
+
+       return -1;
+}
+
+int run_command_timeout(char *path, char *args[], char *env[], int timeout)
+{
+       int fd = open("/dev/null", O_WRONLY);
+       if (fd < 0) {
+               _E("open /dev/null error: %m");
+               return -1;
+       }
+
+       int res = run_command_write_fd_timeout(path, args, env, fd, NULL, 0, timeout);
+
+       close(fd);
+
+       return res;
 }
 
 static int remove_dir_internal(int fd)

diff --git a/src/shared/util.h b/src/shared/util.h
index 85416fa..eb34a6b 100644 (file)
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -43,7 +43,9 @@ int move_file(char *src, char *dst);
 
 int dump_file_write_fd(char *src, int dfd);
 
-int run_command_write_fd(char *cmd, int dfd);
+int run_command_write_fd_timeout(char *path, char *args[], char *env[], int dfd, char *buff, int size, int timeout);
+
+int run_command_timeout(char *path, char *args[], char *env[], int timeout);
 
 int remove_dir(const char *path, int del_dir);