在脚本路径前加上 `/usr/bin/env -i` 是否可以安全地使用 system() 和 popen()？

Question

我现在正在进行代码审查，我被这家伙编写的代码量所震撼，只是为了执行一个脚本（具有硬编码路径且没有输入参数）并从中读取输出。 （顺便说一句，他有很多错误。）

我之前遇到过类似的问题，有人建议我手动执行 pipe/fork/exec 会“更安全”。我知道两个潜在的问题：

1. system() 和 popen() 执行 shell 命令时，可能会将潜在有害的环境变量值传递给以这种方式执行的程序
2. 另一个是从用户输入构造命令时。我可以想象 subshel​​l 做各种有害的事情等等。

我想知道在这种情况下建议使用popen() 是否可行。这将大大简化代码。第二点不是问题，因为没有用户输入。通过在执行脚本之前使用env -i 清理环境应该可以解决第一个问题：

FILE *fp = popen("/usr/bin/env -i /path/to/some/fancy/script.sh", "r");
/* ... */

是否还有其他我遗漏的潜在问题，或者“手动”执行脚本仍然值得付出努力？

Answer 1

从技术上讲，这不是您对如何安全地拨打popen() 的问题的答案，而是您应该问的问题的答案：“如何打造更好的popen()”

函数child_spawn(argv, env, flags) 将建立与子进程通信的管道，并生成子进程。它会返回一个 struct child 保存子 pid 和文件描述符以进行通信。

argv 是 NULL 终止的命令和参数字符串数组，而 env 是 NULL 终止的环境变量字符串数组。如果env 为NULL，则子级将从父级继承环境。

所以argv 应该有这种形式

const char* argv[] = {"/bin/ls", "-l", NULL};

而env 应该有这种形式

const char **env = NULL;

or

const char *env[] = 
   {
      "PATH=/bin:/usr/bin",
      "HOME=/tmp",
      "SHELL=/bin/sh",
      NULL
   };

当您完成子进程时，child_wait() 将关闭与子进程关联的文件描述符并等待它退出。

要使用child_spawn() 代替popen()，您可以这样称呼它：

struct child c = child_spawn(argv, NULL, CHILD_PIPE_STDOUT);

您现在可以阅读 c->fd_out 以获取子标准输出的内容。

c->fd_in、c->fd_out、c->fd_err 中的常量c->fd_out、CHILD_PIPE_STDOUT 和CHILD_PIPE_STDERR 可以“或”-ed 一起具有有效的文件描述符

请注意，如果您使用CHILD_PIPE_STDIN|CHILD_PIPE_STDOUT 生成孩子，则在读取和写入时存在死锁风险，除非您执行非阻塞 io。

函数my_system() 是一个关于如何使用child_spawn() 实现更安全的system() 的示例

/*
  We have to #define _GNU_SOURCE to get access to `char **environ`
*/
#define _GNU_SOURCE
#include <unistd.h>


#include <sys/types.h>
#include <sys/wait.h>
#include <sys/sendfile.h>
#include <stdio.h>
#include <stdlib.h>
#include <error.h>
#include <errno.h>
#include <string.h>


struct child
{
  pid_t pid;
  int fd_in;
  int fd_out;
  int fd_err;
};

static void
close_if_valid(int fd)
{
  if (fd != -1) close(fd);
}

/* 
   Closes all file-descriptors for child communication
   and waits for child to exit

   returns status value from waitpid().
   see `man waitpid` on how to interpret that value
 */
int child_wait(struct child *c)
{
  close_if_valid(c->fd_in);
  close_if_valid(c->fd_out);
  close_if_valid(c->fd_err);

  int status;

  pid_t p = waitpid(c->pid, &status, 0);
  if (p == 0)
    error(1, errno, "waitpid() failed");

  return status; 
}


int
dup_if_valid(int fd1, int fd2)
{
  if (fd1 != -1 && fd1 != fd2)
    return dup2(fd1, fd2);
  return fd2;
}


pid_t
child_spawn_fd(const char *const argv[], const char *const env[],
           int in, int out, int err)
{
  fflush(stdout);
  pid_t p = fork();

  if (p)
    return p;

  /***********************
    We are now in child
  ***********************/

  /* 
     Set file descriptors to expected values,
     -1 means inherit from parent
  */
  if (dup_if_valid(in, 0) == -1)
    goto CHILD_ERR;

  if (dup_if_valid(out, 1) == -1)
    goto CHILD_ERR;

  if (dup_if_valid(err, 2) == -1)
    goto CHILD_ERR;

  /*
    close all unneeded file descriptors
    This will free resources and keep files and sockets belonging to
    the parent from beeing open longer than needed

    On *BSD we may call `closefrom(3);`, but this may not exits
    on Linux. So we loop over all possible file descriptor numbers.
    A better solution, is to look in `/proc/self/fs`
  */
  int max_fd = sysconf(_SC_OPEN_MAX);

  for (int fd = 3; fd <= max_fd; fd++)
    close(fd);

  if (env)
    environ = (char **)env;

  /* Change to execvp if command should be looked up in $PATH */
  execv(argv[0], (char * const *)argv);

 CHILD_ERR:
  _exit(1);
}


#define CHILD_PIPE_STDIN (1 << 0)
#define CHILD_PIPE_STDOUT (1 << 1)
#define CHILD_PIPE_STDERR (1 << 2)

#define READ_END 0
#define WRITE_END 1


struct child
child_spawn(const char * const argv[], const char * const env[], int flags)
{
  int in_pipe[2] = {-1, -1};
  int out_pipe[2] = {-1, -1};
  int err_pipe[2] = {-1, -1};

  if (flags & CHILD_PIPE_STDIN)
    if (pipe(in_pipe))
      error(EXIT_FAILURE, errno, "pipe(in_pipe) failed");

  if (flags & CHILD_PIPE_STDOUT)
    if (pipe(out_pipe))
      error(EXIT_FAILURE, errno, "pipe(out_pipe) failed");

  if (flags & CHILD_PIPE_STDERR)
    if (pipe(err_pipe))
      error(EXIT_FAILURE, errno, "pipe(err_pipe) failed");

  pid_t p = child_spawn_fd(argv, env,
               in_pipe[READ_END],
               out_pipe[WRITE_END],
               err_pipe[WRITE_END]);

  if (p == -1)
    error(EXIT_FAILURE, errno, "fork() failed");

  close_if_valid(in_pipe[READ_END]);
  close_if_valid(out_pipe[WRITE_END]);
  close_if_valid(err_pipe[WRITE_END]);

  struct child c =
    {
      .pid = p,
      .fd_in = in_pipe[WRITE_END],
      .fd_out = out_pipe[READ_END],
      .fd_err = err_pipe[READ_END],
    };

  return c;
}

/* 
   Safer implementation of `system()`. It does not invoke shell, and takes
   command as NULL terminated list of execuatable and parameters
*/
int
my_system(const char * const argv[])
{
  struct child c = child_spawn(argv, NULL, 0);

  int status = child_wait(&c);

  if (WIFEXITED(status))
    return WEXITSTATUS(status);
  else
    return -1;
}




int
main (int argc, char **argv)
{
  printf("Running 'ls -l' using my_system()\n"); 
  printf("---------------------------------\n");
  fflush(stdout);

  const char * ls_argv[] =
    {
      "/bin/ls",
      "-l",
      NULL
    };

  int e = my_system(ls_argv);
  printf("---------\n");
  printf("\exit code ---> %d\n", e); 



  printf("\nRunning 'ls -l' using child_spawn() and reading from stdout\n"); 
  printf("-----------------------------------------------------------\n");
  fflush(stdout);

  struct child c = child_spawn(ls_argv, NULL, CHILD_PIPE_STDOUT);

  /* 
     Read from the childs stdout and write to current stdout
  */
  size_t copied = 0;
  while (1)
    {
      char buff[4096];

      ssize_t rlen = read(c.fd_out, buff, 4096);
      if (rlen == -1)
    error(EXIT_FAILURE, errno, "read() failed");

      if (rlen == 0)
    break;

      size_t written = 0;
      while (written < rlen)
    {
      ssize_t wlen = write(1, buff + written, rlen - written);
      if (wlen == -1)
        error(EXIT_FAILURE, errno, "write() failed");

      written += wlen;
    }
      copied += written;
    }


  /* Wait for child to end */
  int status = child_wait(&c);

  printf("---------\n");

  if (WIFEXITED(status))
    {  
      printf("  ---> child exited normally with exit code %d and with %ld bytes copied\n",
         WEXITSTATUS(status),
         copied);
    }
  else
    printf("  ---> child exited by som other reason than _exit()");


  printf("\nWriting to Elmer Fudd filter\n"); 
  const char *quote = "Be very very quiet, I'm hunting rabbits!\n";

  printf("Original text: %s", quote);

  printf("-----------------------------------------------------------\n");
  fflush(stdout);

  const char *fudd_filter[] =
    {"/bin/sed", "-e" "s/r/w/g", NULL};  

  struct child c2 = child_spawn(fudd_filter, NULL, CHILD_PIPE_STDIN);
  size_t qlen = strlen(quote);
  const char *q = quote;

  while (qlen)
    {
      ssize_t wlen = write(c2.fd_in, q, qlen);
      if (wlen == -1)
    error(EXIT_FAILURE, errno, "write() failed");

      q += wlen;
      qlen -= wlen;
    }

  child_wait(&c2);
}