[rt-devel] Better handling of sendmail (Re: Bugfix for security patch on mod_perl)

[paul.szabo at sydney.edu.au]

I feel that the invocation of sendmail in RT/Interface/Email.pm is
"wrong":
 - uses IPC::Open2 instead of plain open($mail,"| $path @args") though
   it never attempts to read STDOUT, loses STDERR without Open3
 - uses pipe which can have consequences for invoker, including losing
   the exit status or output of subprocess
I propose a more standard, simpler, handling of the sendmail process,
in a way which captures the exit status and all the STDERR or STDOUT
output.
Patch below is against the Debian 3.8 version (I believe it would go
cleanly into 4.0 also). Please see
http://bugs.debian.org/674865
also.

Cheers, Paul

Paul Szabo   psz at maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia



# diff -u /usr/share/request-tracker3.8/lib/RT/Interface/Email.pm.bak /usr/share/request-tracker3.8/lib/RT/Interface/Email.pm
--- /usr/share/request-tracker3.8/lib/RT/Interface/Email.pm.bak 2012-05-19 22:37:07.000000000 +1000
+++ /usr/share/request-tracker3.8/lib/RT/Interface/Email.pm     2012-05-29 06:40:37.000000000 +1000
@@ -443,28 +443,40 @@
         }
 
         eval {
-            # don't ignore CHLD signal to get proper exit code
-            local $SIG{'CHLD'} = 'DEFAULT';
-
-            # if something wrong with $mail->print we will get PIPE signal, handle it
-            local $SIG{'PIPE'} = sub { die "program unexpectedly closed pipe" };
-
-            require IPC::Open2;
-            my ($mail, $stdout);
-            my $pid = IPC::Open2::open2( $stdout, $mail, $path, @args )
-                or die "couldn't execute program: $!";
-
-            $args{'Entity'}->print($mail);
-            close $mail or die "close pipe failed: $!";
-
-            waitpid($pid, 0);
-            if ($?) {
-                # sendmail exit statuses mostly errors with data not software
-                # TODO: status parsing: core dump, exit on signal or EX_*
-                my $msg = "$msgid: `$path @args` exited with code ". ($?>>8);
-                $msg = ", interrupted by signal ". ($?&127) if $?&127;
-                $RT::Logger->error( $msg );
-            }
+            ## don't ignore CHLD signal to get proper exit code
+            #local $SIG{'CHLD'} = 'DEFAULT';
+            #
+            ## if something wrong with $mail->print we will get PIPE signal, handle it
+            #local $SIG{'PIPE'} = sub { die "program unexpectedly closed pipe" };
+            #
+            #require IPC::Open2;
+            #my ($mail, $stdout);
+            #my $pid = IPC::Open2::open2( $stdout, $mail, $path, @args )
+            #    or die "couldn't execute program: $!";
+            #
+            #$args{'Entity'}->print($mail);
+            #close $mail or die "close pipe failed: $!";
+            #
+            #waitpid($pid, 0);
+            #if ($?) {
+            #    # sendmail exit statuses mostly errors with data not software
+            #    # TODO: status parsing: core dump, exit on signal or EX_*
+            #    my $msg = "$msgid: `$path @args` exited with code ". ($?>>8);
+            #    $msg = ", interrupted by signal ". ($?&127) if $?&127;
+            #    $RT::Logger->error( $msg );
+            #}
+            #####
+            #use File::Temp;    # Done above
+            my $tmp_fh = File::Temp->new() or die "Cannot create temp file for sendmail data\n";
+            my $tmp_fn = $tmp_fh->filename or die "Nameless temp file for sendmail data\n";
+            $args{'Entity'}->print($tmp_fh) or die "Cannot write temp file for sendmail data\n";
+            close ($tmp_fh) or die "Cannot close temp file for sendmail data\n";
+            my $cmd = "$path @args < $tmp_fn 2>&1";
+            #$RT::Logger->info( "PSz using command: $cmd" );
+            my $msg = `$cmd`;
+            $? and $msg .= "sendmail status: $? (exit code " . ($?>>8) . ", signal " . ($?&127) .")";
+            $msg and die "sendmail output: $msg\n";
+            unlink ($tmp_fn) or die "Cannot delete temp file for sendmail data\n";
         };
         if ( $@ ) {
             $RT::Logger->crit( "$msgid: Could not send mail with command `$path @args`: " . $@ );