Attachment 16953 Details for Bug 51659

[patch] the fix ffrom Derek

03cvs-client-exploit-fix.diff (text/plain), 10.52 KB, created by Sebastian Krahmer on 2004-03-22 17:44:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Sebastian Krahmer

Created: 2004-03-22 17:44:04 UTC

Size: 10.52 KB

patch

obsolete

>Index: src/client.c
>===================================================================
>RCS file: /cvs/ccvs/src/client.c,v
>retrieving revision 1.318.4.12
>diff -u -p -r1.318.4.12 client.c
>--- src/client.c	25 Feb 2004 06:46:15 -0000	1.318.4.12
>+++ src/client.c	18 Mar 2004 21:04:46 -0000
>@@ -1045,6 +1045,19 @@ call_in_directory (pathname, func, data)
>     int reposdirname_absolute;
>     int newdir = 0;
> 
>+    /* For security reasons, if PATHNAME is absolute or attemps to ascend
>+     * outside of the current sanbbox, we abort.  The server should not send us
>+     * anything but relative paths which remain inside the sandbox here.
>+     * Anything less means a trojan CVS server could create and edit arbitrary
>+     * files on the client.
>+     */
>+    if (isabsolute (pathname) || pathname_levels (pathname) > 0)
>+    {
>+	error (0, 0,
>+               "Server attempted to update a file via an invalid pathname:");
>+        error (1, 0, "`%s'.", pathname);
>+    }
>+
>     reposname = NULL;
>     read_line (&reposname);
>     assert (reposname != NULL);
>Index: src/import.c
>===================================================================
>RCS file: /cvs/ccvs/src/import.c,v
>retrieving revision 1.133.4.4
>diff -u -p -r1.133.4.4 import.c
>--- src/import.c	4 Mar 2004 02:59:43 -0000	1.133.4.4
>+++ src/import.c	18 Mar 2004 21:04:48 -0000
>@@ -163,8 +163,8 @@ import (argc, argv)
>      * to call it from anywhere else.
>      */
>     if ((cp = strstr(argv[0], "CVS")) &&   /* path contains "CVS" AND ... */
>-        ((cp == argv[0]) || (*(cp-1) == '/')) && /* /^CVS/ OR m#/CVS# AND ... */
>-        ((*(cp+3) == '\0') || (*(cp+3) == '/')) /* /CVS$/ OR m#CVS/# */
>+        ((cp == argv[0]) || ISDIRSEP(*(cp-1))) && /* /^CVS/ OR m#/CVS# AND ... */
>+        ((*(cp+3) == '\0') || ISDIRSEP(*(cp+3))) /* /CVS$/ OR m#CVS/# */
>        )
>     {
>         error (0, 0,
>@@ -183,8 +183,7 @@ import (argc, argv)
>     }
> 
>     /* XXX - this should be a module, not just a pathname */
>-    if (! isabsolute (argv[0])
>-	&& pathname_levels (argv[0]) == 0)
>+    if (!isabsolute (argv[0]) && pathname_levels (argv[0]) == 0)
>     {
> 	if (current_parsed_root == NULL)
> 	{
>Index: src/sanity.sh
>===================================================================
>RCS file: /cvs/ccvs/src/sanity.sh,v
>retrieving revision 1.752.2.98
>diff -u -p -r1.752.2.98 sanity.sh
>--- src/sanity.sh	14 Mar 2004 07:29:00 -0000	1.752.2.98
>+++ src/sanity.sh	18 Mar 2004 21:05:10 -0000
>@@ -25988,25 +25988,33 @@ ${PROG} \[diff aborted\]: read lock fail
> "${testcvs} -d ${CVSROOT1} -q co ../root2/dir2" \
> "${PROG} checkout: in directory \.\./root2/dir2:
> ${PROG} checkout: .\.\..-relative repositories are not supported.
>-${PROG} \[checkout aborted\]: illegal source repository"
>+${PROG} \[checkout aborted\]: illegal source repository" \
>+"$PROG checkout: Server attempted to update a file via an invalid pathname:
>+$PROG \[checkout aborted\]: \`\.\./root2/'\."
> 	  rm -rf ../root2
> 	  dotest_fail multiroot3-13 \
> "${testcvs} -d ${CVSROOT2} -q co ../root1/dir1" \
> "${PROG} checkout: in directory \.\./root1/dir1:
> ${PROG} checkout: .\.\..-relative repositories are not supported.
>-${PROG} \[checkout aborted\]: illegal source repository"
>+${PROG} \[checkout aborted\]: illegal source repository" \
>+"$PROG checkout: Server attempted to update a file via an invalid pathname:
>+$PROG \[checkout aborted\]: \`\.\./root1/'\."
> 	  rm -rf ../root1
> 	  dotest_fail multiroot3-14 \
> "${testcvs} -d ${CVSROOT1} -q co ./../root2/dir2" \
> "${PROG} checkout: in directory \./\.\./root2/dir2:
> ${PROG} checkout: .\.\..-relative repositories are not supported.
>-${PROG} \[checkout aborted\]: illegal source repository"
>+${PROG} \[checkout aborted\]: illegal source repository" \
>+"$PROG checkout: Server attempted to update a file via an invalid pathname:
>+$PROG \[checkout aborted\]: \`\./\.\./root2/'\."
> 	  rm -rf ../root2
> 	  dotest_fail multiroot3-15 \
> "${testcvs} -d ${CVSROOT2} -q co ./../root1/dir1" \
> "${PROG} checkout: in directory \./\.\./root1/dir1:
> ${PROG} checkout: .\.\..-relative repositories are not supported.
>-${PROG} \[checkout aborted\]: illegal source repository"
>+${PROG} \[checkout aborted\]: illegal source repository" \
>+"$PROG checkout: Server attempted to update a file via an invalid pathname:
>+$PROG \[checkout aborted\]: \`\./\.\./root1/'\."
> 	  rm -rf ../root1
> 
> 	  cd ../..
>@@ -27045,10 +27053,204 @@ u=rw,g=r,o=r
> abc
> update"
> 
>+	    # The following test tests what was a potential client update in
>+	    # CVS versions 1.11.14 and CVS versions 1.12.6 and earlier.  This
>+	    # exploit would allow a trojan server to create arbitrary files,
>+	    # anywhere the user had write permissions, even outside of the
>+	    # user's sandbox.
>+	    cat >$HOME/.bashrc <<EOF
>+#!$TESTSHELL
>+# This is where login scripts would usually be
>+# stored.
>+EOF
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Rcs-diff $HOME/"
>+echo "$HOME/.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "20"
>+echo "a1 1"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    
>+	    # If I don't run the following sleep between the above cat and
>+	    # the following calls to dotest, sometimes the serveme file isn't
>+	    # completely written yet by the time CVS tries to execute it,
>+	    # causing the shell to intermittantly report syntax errors (usually
>+	    # early EOF).  There's probably a new race condition here, but this
>+	    # works.
>+	    #
>+	    # Incidentally, I can reproduce this behavior with Linux 2.4.20 and
>+	    # Bash 2.05 or Bash 2.05b.
>+	    sleep 1
>+	    dotest_fail client-10 "$testcvs update" \
>+"$PROG update: Server attempted to update a file via an invalid pathname:
>+$PROG \[update aborted\]: \`$HOME/'\."
>+
>+	    # A second try at a client exploit.  This one never actually
>+	    # failed in the past, but I thought it wouldn't hurt to add a test.
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Rcs-diff ./"
>+echo "$HOME/.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "20"
>+echo "a1 1"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-11 "$testcvs update" \
>+"$PROG \[update aborted\]: patch original file \./\.bashrc does not exist"
>+
>+	    # A third try at a client exploit.  This one did used to fail like
>+	    # client-10.
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Rcs-diff ../../home/"
>+echo "../../.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "20"
>+echo "a1 1"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-12 "$testcvs update" \
>+"$PROG update: Server attempted to update a file via an invalid pathname:
>+$PROG \[update aborted\]: \`\.\./\.\./home/'\."
>+
>+	    # Try the same exploit using the Created response.
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Created $HOME/"
>+echo "$HOME/.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "26"
>+echo "#! /bin/sh"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-13 "$testcvs update" \
>+"$PROG update: Server attempted to update a file via an invalid pathname:
>+$PROG \[update aborted\]: \`$HOME/'\."
>+
>+	    # Now try using the Update-existing response
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Update-existing ../../home/"
>+echo "../../home/.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "26"
>+echo "#! /bin/sh"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-14 "$testcvs update" \
>+"$PROG update: Server attempted to update a file via an invalid pathname:
>+$PROG \[update aborted\]: \`\.\./\.\./home/'\."
>+
>+	    # Try the same exploit using the Merged response.
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Merged $HOME/"
>+echo "$HOME/.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "26"
>+echo "#! /bin/sh"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-15 "$testcvs update" \
>+"$PROG update: Server attempted to update a file via an invalid pathname:
>+$PROG \[update aborted\]: \`$HOME/'\."
>+
>+	    # Now try using the Updated response
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Updated ../../home/"
>+echo "../../home/.bashrc"
>+echo "/.bashrc/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "26"
>+echo "#! /bin/sh"
>+echo "echo 'gotcha!'"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-16 "$testcvs update" \
>+"$PROG update: Server attempted to update a file via an invalid pathname:
>+$PROG \[update aborted\]: \`\.\./\.\./home/'\."
>+
>+	    # Try the same exploit using the Copy-file response.
>+	    # As far as I know, Copy-file was never exploitable either.
>+	    cat >$TESTDIR/serveme <<EOF
>+#!$TESTSHELL
>+echo "Valid-requests Root Valid-responses valid-requests Directory Entry Modified Unchanged Argument Argumentx ci co update"
>+echo "ok"
>+echo "Created ."
>+echo "./innocuous"
>+echo "/innocuous/73.50///"
>+echo "u=rw,g=rw,o=rw"
>+echo "26"
>+echo "#! /bin/sh"
>+echo "echo 'gotcha!'"
>+echo "Copy-file ."
>+echo "./innocuous"
>+echo "$HOME/innocuous"
>+echo "ok"
>+cat >/dev/null
>+EOF
>+	    sleep 1
>+	    dotest_fail client-18 "$testcvs update" \
>+"$PROG \[update aborted\]: protocol error: Copy-file tried to specify directory"
>+
>+	    # And verify that none of the exploits was successful.
>+	    dotest client-19 "cat $HOME/.bashrc" \
>+"#!$TESTSHELL
>+# This is where login scripts would usually be
>+# stored\."
>+
>+	    if $keep; then
>+	      echo Keeping $TESTDIR and exiting due to --keep
>+	      exit 0
>+	    fi
>+
> 	    cd ../..
> 	    rm -r 1
> 	    rmdir ${TESTDIR}/bogus
>-	    rm ${TESTDIR}/serveme
>+	    rm $TESTDIR/serveme $HOME/.bashrc
> 	    CVS_SERVER=${save_CVS_SERVER}; export CVS_SERVER
> 	  fi # skip the whole thing for local
> 	  ;;

Actions: View | Diff

Attachments on bug 51659: 16953 | 17910 | 17911 | 18205 | 18206