spamass-milter/popen.diff

   1 Index: spamass-milter.cpp
   2 ===================================================================
   3 RCS file: /cvsroot/spamass-milt/spamass-milt/spamass-milter.cpp,v
   4 retrieving revision 1.91
   5 diff -u -r1.91 spamass-milter.cpp
   6 --- spamass-milter.cpp  24 Jul 2006 19:59:17 -0000      1.91
   7 +++ spamass-milter.cpp  10 Mar 2010 18:52:22 -0000
   8 @@ -171,10 +171,6 @@
   9  bool flag_expand = false;      /* alias/virtusertable expansion */
  10  bool warnedmacro = false;      /* have we logged that we couldn't fetch a macro? */
  11
  12 -#if defined(__FreeBSD__) /* popen bug - see PR bin/50770 */
  13 -static pthread_mutex_t popen_mutex = PTHREAD_MUTEX_INITIALIZER;
  14 -#endif
  15 -
  16  // {{{ main()
  17
  18  int
  19 @@ -461,59 +457,24 @@
  20                            send another copy.  The milter API will not let you send the
  21                            message AND return a failure code to the sender, so this is
  22                            the only way to do it. */
  23 -#if defined(__FreeBSD__)
  24 -                       int rv;
  25 -#endif
  26 -
  27 -#if defined(HAVE_ASPRINTF)
  28 -                       char *buf;
  29 -#else
  30 -                       char buf[1024];
  31 -#endif
  32 -                       char *fmt="%s \"%s\"";
  33 +                       char *popen_argv[3];
  34                         FILE *p;
  35
  36 -#if defined(HAVE_ASPRINTF)
  37 -                       asprintf(&buf, fmt, SENDMAIL, spambucket);
  38 -#else
  39 -#if defined(HAVE_SNPRINTF)
  40 -                       snprintf(buf, sizeof(buf)-1, fmt, SENDMAIL, spambucket);
  41 -#else
  42 -                       /* XXX possible buffer overflow here */
  43 -                       sprintf(buf, fmt, SENDMAIL, spambucket);
  44 -#endif
  45 -#endif
  46 -
  47 -                       debug(D_COPY, "calling %s", buf);
  48 -#if defined(__FreeBSD__) /* popen bug - see PR bin/50770 */
  49 -                       rv = pthread_mutex_lock(&popen_mutex);
  50 -                       if (rv)
  51 -                       {
  52 -                               debug(D_ALWAYS, "Could not lock popen mutex: %s", strerror(rv));
  53 -                               abort();
  54 -                       }
  55 -#endif
  56 -                       p = popen(buf, "w");
  57 +                       popen_argv[0] = SENDMAIL;
  58 +                       popen_argv[1] = spambucket;
  59 +                       popen_argv[2] = NULL;
  60 +
  61 +                       debug(D_COPY, "calling %s %s", SENDMAIL, spambucket);
  62 +                       p = popenv(popen_argv, "w");
  63                         if (!p)
  64                         {
  65 -                               debug(D_COPY, "popen failed(%s).  Will not send a copy to spambucket", strerror(errno));
  66 +                               debug(D_COPY, "popenv failed(%s).  Will not send a copy to spambucket", strerror(errno));
  67                         } else
  68                         {
  69                                 // Send message provided by SpamAssassin
  70                                 fwrite(assassin->d().c_str(), assassin->d().size(), 1, p);
  71 -                               pclose(p); p = NULL;
  72 +                               fclose(p); p = NULL;
  73                         }
  74 -#if defined(__FreeBSD__)
  75 -                       rv = pthread_mutex_unlock(&popen_mutex);
  76 -                       if (rv)
  77 -                       {
  78 -                               debug(D_ALWAYS, "Could not unlock popen mutex: %s", strerror(rv));
  79 -                               abort();
  80 -                       }
  81 -#endif
  82 -#if defined(HAVE_ASPRINTF)
  83 -                       free(buf);
  84 -#endif
  85                 }
  86                 return SMFIS_REJECT;
  87         }
  88 @@ -842,30 +803,19 @@
  89                 /* open a pipe to sendmail so we can do address expansion */
  90
  91                 char buf[1024];
  92 -               char *fmt="%s -bv \"%s\" 2>&1";
  93 -
  94 -#if defined(HAVE_SNPRINTF)
  95 -               snprintf(buf, sizeof(buf)-1, fmt, SENDMAIL, envrcpt[0]);
  96 -#else
  97 -               /* XXX possible buffer overflow here */
  98 -               sprintf(buf, fmt, SENDMAIL, envrcpt[0]);
  99 -#endif
 100 +               char *popen_argv[4];
 101 +
 102 +               popen_argv[0] = SENDMAIL;
 103 +               popen_argv[1] = "-bv";
 104 +               popen_argv[2] = envrcpt[0];
 105 +               popen_argv[3] = NULL;
 106
 107 -               debug(D_RCPT, "calling %s", buf);
 108 +               debug(D_RCPT, "calling %s -bv %s", SENDMAIL, envrcpt[0]);
 109
 110 -#if defined(__FreeBSD__) /* popen bug - see PR bin/50770 */
 111 -               rv = pthread_mutex_lock(&popen_mutex);
 112 -               if (rv)
 113 -               {
 114 -                       debug(D_ALWAYS, "Could not lock popen mutex: %s", strerror(rv));
 115 -                       abort();
 116 -               }
 117 -#endif
 118 -
 119 -               p = popen(buf, "r");
 120 +               p = popenv(popen_argv, "r");
 121                 if (!p)
 122                 {
 123 -                       debug(D_RCPT, "popen failed(%s).  Will not expand aliases", strerror(errno));
 124 +                       debug(D_RCPT, "popenv failed(%s).  Will not expand aliases", strerror(errno));
 125                         assassin->expandedrcpt.push_back(envrcpt[0]);
 126                 } else
 127                 {
 128 @@ -890,16 +840,8 @@
 129                                         assassin->expandedrcpt.push_back(p+7);
 130                                 }
 131                         }
 132 -                       pclose(p); p = NULL;
 133 +                       fclose(p); p = NULL;
 134                 }
 135 -#if defined(__FreeBSD__)
 136 -               rv = pthread_mutex_unlock(&popen_mutex);
 137 -               if (rv)
 138 -               {
 139 -                       debug(D_ALWAYS, "Could not unlock popen mutex: %s", strerror(rv));
 140 -                       abort();
 141 -               }
 142 -#endif
 143         } else
 144         {
 145                 assassin->expandedrcpt.push_back(envrcpt[0]);
 146 @@ -2157,5 +2099,71 @@
 147         warnedmacro = true;
 148  }
 149
 150 +/*
 151 +   untrusted-argument-safe popen function - only supports "r" and "w" modes
 152 +   for simplicity, and always reads stdout and stderr in "r" mode.  Call
 153 +   fclose to close the FILE.
 154 +*/
 155 +FILE *popenv(char *const argv[], const char *type)
 156 +{
 157 +       FILE *iop;
 158 +       int pdes[2];
 159 +       int save_errno;
 160 +       if ((*type != 'r' && *type != 'w') || type[1])
 161 +       {
 162 +               errno = EINVAL;
 163 +               return (NULL);
 164 +       }
 165 +       if (pipe(pdes) < 0)
 166 +               return (NULL);
 167 +       switch (fork()) {
 168 +
 169 +       case -1:                        /* Error. */
 170 +               save_errno = errno;
 171 +               (void)close(pdes[0]);
 172 +               (void)close(pdes[1]);
 173 +               errno = save_errno;
 174 +               return (NULL);
 175 +               /* NOTREACHED */
 176 +       case 0:                         /* Child. */
 177 +               if (*type == 'r') {
 178 +                       /*
 179 +                        * The dup2() to STDIN_FILENO is repeated to avoid
 180 +                        * writing to pdes[1], which might corrupt the
 181 +                        * parent's copy.  This isn't good enough in
 182 +                        * general, since the exit() is no return, so
 183 +                        * the compiler is free to corrupt all the local
 184 +                        * variables.
 185 +                        */
 186 +                       (void)close(pdes[0]);
 187 +                       (void)dup2(pdes[1], STDOUT_FILENO);
 188 +                       (void)dup2(pdes[1], STDERR_FILENO);
 189 +                       if (pdes[1] != STDOUT_FILENO && pdes[1] != STDERR_FILENO) {
 190 +                               (void)close(pdes[1]);
 191 +                       }
 192 +               } else {
 193 +                       if (pdes[0] != STDIN_FILENO) {
 194 +                               (void)dup2(pdes[0], STDIN_FILENO);
 195 +                               (void)close(pdes[0]);
 196 +                       }
 197 +                       (void)close(pdes[1]);
 198 +               }
 199 +               execv(argv[0], argv);
 200 +               exit(127);
 201 +               /* NOTREACHED */
 202 +       }
 203 +
 204 +       /* Parent; assume fdopen can't fail. */
 205 +       if (*type == 'r') {
 206 +               iop = fdopen(pdes[0], type);
 207 +               (void)close(pdes[1]);
 208 +       } else {
 209 +               iop = fdopen(pdes[1], type);
 210 +               (void)close(pdes[0]);
 211 +       }
 212 +
 213 +       return (iop);
 214 +}
 215 +
 216  // }}}
 217  // vim6:ai:noexpandtab
 218 Index: spamass-milter.h
 219 ===================================================================
 220 RCS file: /cvsroot/spamass-milt/spamass-milt/spamass-milter.h,v
 221 retrieving revision 1.23
 222 diff -u -r1.23 spamass-milter.h
 223 --- spamass-milter.h    7 Apr 2005 02:04:24 -0000       1.23
 224 +++ spamass-milter.h    10 Mar 2010 18:52:22 -0000
 225 @@ -186,5 +186,6 @@
 226  void parse_debuglevel(char* string);
 227  char *strlwr(char *str);
 228  void warnmacro(char *macro, char *scope);
 229 +FILE *popenv(char *const argv[], const char *type);
 230
 231  #endif