cvsserver: Make req_Root more critical of its input data

The path submitted with the Root request has to be absolute
(cvs does it this way and it may save us some sanity checks
later)

If multiple roots are specified (e.g. because we use
pserver authentication which will already include the
root), ensure that they say all the same.

Probably neither is a security risk, and neither should ever
be triggered by a sane client, but when validating
input data, it's better to be save than sorry.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 2b4825a8eebad57fa9857593d0a8aa5928b313fe..d41b29f30b6f07429b227fc4e07fffdcb292bbee 100755 (executable)
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -167,6 +167,17 @@ sub req_Root
     my ( $cmd, $data ) = @_;
     $log->debug("req_Root : $data");
 
+    unless ($data =~ m#^/#) {
+       print "error 1 Root must be an absolute pathname\n";
+       return 0;
+    }
+
+    if ($state->{CVSROOT}
+       && ($state->{CVSROOT} ne $data)) {
+       print "error 1 Conflicting roots specified\n";
+       return 0;
+    }
+
     $state->{CVSROOT} = $data;
 
     $ENV{GIT_DIR} = $state->{CVSROOT} . "/";

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index e9ef315173f9596212d9104587486315603b2b51..41dcf646d132e35ca37ab6cc75fc2a7e70cc765e 100755 (executable)
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -110,6 +110,40 @@ test_expect_success 'pserver authentication failure (login/non-anonymous user)'
    tail -n1 log | grep -q "^I HATE YOU$"'
 
 
+# misuse pserver authentication for testing of req_Root
+
+cat >request-relative  <<EOF
+BEGIN AUTH REQUEST
+gitcvs.git
+anonymous
+
+END AUTH REQUEST
+EOF
+
+cat >request-conflict  <<EOF
+BEGIN AUTH REQUEST
+$SERVERDIR
+anonymous
+
+END AUTH REQUEST
+Root $WORKDIR
+EOF
+
+test_expect_success 'req_Root failure (relative pathname)' \
+  'if cat request-relative | git-cvsserver pserver >log 2>&1
+   then
+       echo unexpected success
+       false
+   else
+       true
+   fi &&
+   tail log | grep -q "^error 1 Root must be an absolute pathname$"'
+
+test_expect_success 'req_Root failure (conflicting roots)' \
+  'cat request-conflict | git-cvsserver pserver >log 2>&1 &&
+   tail log | grep -q "^error 1 Conflicting roots specified$"'
+
+
 #--------------
 # CONFIG TESTS
 #--------------