svn commit: rev 51851 - spamassassin/branches/3.0/lib/Mail/SpamAssassin

[jm...@apache.org]

Author: jm
Date: Sun Oct  3 14:30:32 2004
New Revision: 51851

Modified:
   spamassassin/branches/3.0/lib/Mail/SpamAssassin/Util.pm
Log:
bug 3649: helper_app_pipe_open doesn't do the right thing if STD{IN,OUT,ERR} filehandles != {0,1,2} fds

Modified: spamassassin/branches/3.0/lib/Mail/SpamAssassin/Util.pm
==============================================================================
--- spamassassin/branches/3.0/lib/Mail/SpamAssassin/Util.pm	(original)
+++ spamassassin/branches/3.0/lib/Mail/SpamAssassin/Util.pm	Sun Oct  3 14:30:32 2004
@@ -41,6 +41,7 @@
 use Time::Local;
 use Sys::Hostname (); # don't import hostname() into this namespace!
 use Fcntl;
+use POSIX (); # don't import anything unless we ask explicitly!
 
 ###########################################################################
 
@@ -1016,14 +1017,58 @@
   setuid_to_euid();
   dbg ("setuid: helper proc $$: ruid=$< euid=$>");
 
-  if ($stdinfile) {              # < $tmpfile
-    close STDIN;
-    open (STDIN, "<$stdinfile") or die "cannot open $stdinfile: $!";
-  }
+  # now set up the fds.  due to some wierdness, we may have to ensure that we
+  # *really* close the correct fd number, since some other code may have
+  # redirected the meaning of STDOUT/STDIN/STDERR it seems... (bug 3649). use
+  # POSIX::close() for that. it's safe to call close() and POSIX::close() on
+  # the same fd; the latter is a no-op in that case.
+
+  if (!$stdinfile) {              # < $tmpfile
+    # ensure we have *some* kind of fd 0.
+    $stdinfile = "/dev/null";
+  }
+
+  my $f = fileno(STDIN);
+  close STDIN;
+
+  # sanity: was that the *real* STDIN? if not, close that one too ;)
+  if ($f != 0) {
+    POSIX::close(0);
+  }
+  open STDIN, "<$stdinfile" or die "cannot open $stdinfile: $!";
+
+  # this should be impossible; if we just closed fd 0, UNIX
+  # fd behaviour dictates that the next fd opened (the new STDIN)
+  # will be the lowest unused fd number, which should be 0.
+  # so die with a useful error if this somehow isn't the case.
+  if (fileno(STDIN) != 0) {
+    die "setuid: oops: fileno(STDIN) [".fileno(STDIN)."] != 0";
+  }
+
+  # ensure STDOUT is open.  since we just created a pipe to ensure this, it has
+  # to be open to that pipe, and if it isn't, something's seriously screwy.
+  # Update: actually, this fails! see bug 3649 comment 37.  For some reason,
+  # fileno(STDOUT) can be 0; possibly because open("-|") didn't change the fh
+  # named STDOUT, instead changing fileno(1) directly.  So this is now
+  # commented.
+  # if (fileno(STDOUT) != 1) {
+  # die "setuid: oops: fileno(STDOUT) [".fileno(STDOUT)."] != 1";
+  # }
 
   if ($duperr2out) {             # 2>&1
+    my $f = fileno(STDERR);
     close STDERR;
+
+    # sanity: was that the *real* STDERR? if not, close that one too ;)
+    if ($f != 2) {
+      POSIX::close(2);
+    }
     open STDERR, ">&STDOUT" or die "dup STDOUT failed: $!";
+
+    # STDERR must be fd 2 to be useful to subprocesses! (bug 3649)
+    if (fileno(STDERR) != 2) {
+      die "setuid: oops: fileno(STDERR) [".fileno(STDERR)."] != 2";
+    }
   }
 
   exec @cmdline;