www/security/CVE-2007-3846-advisory.txt

  On some platforms (e.g., Win32), svn client can create files in bad places

Summary:
========

  This vulnerability requires prior write access to the repository.

  In Subversion 1.4.4 and earlier versions, on platforms where the
  directory separator is "\" (or anything other than "/"), the client
  libraries can allow files outside the working copy to be created
  during a checkout or update.  This could, in theory, be used to
  place arbitrary code at arbitrary locations on the client machine,
  for example, in system startup scripts.

Known vulnerable:
=================

  Subversion clients <= 1.4.4 (including clients like TortoiseSVN)

Known fixed:
============

  Subversion 1.4.5

  (Search for "Patch" below to see the patch from 1.4.4 -> 1.4.5.
   Search for "Recommendation" to get URLs for the 1.4.5 release.)

Details:
========

  The Subversion client libraries fail to validate that filenames
  obtained from the Subversion server during checkout do not contain
  "..\".  This allows the creation of files outside the checkout
  directory.  Users on operating systems where "\" is not used to
  separate directory paths can commit files with "..\" in the path.
  When these files are checked out onto systems where "\" is a
  directory separator, hilarity may ensue.  To reproduce: 

  On a UNIX system, create a file "..\DIRNAME/exploit.exe" and check
  it into a repository on the top level.  Then check out that
  repository to a Win32 system.  The file will appear outside of the
  checkout directory and instead under "DIRNAME".

Severity:
=========

  Med (arbitrary file creation on client, possibly over system startup files)

  An adversary with write access to the repository could create a file
  at an arbitrary path on the victim's machines.  This could be used
  to install code on the system, for example by placing executable
  code into the startup sequence.

  The attacker first requires write access to the repository from
  which the file will be checked out, and requires that others not
  notice the commit of the dangerous file.  Thus, at first glance it
  might seem that some social engineering is necessary for a full
  exploit.  However, if the repository administrator is the attacker,
  little or no social engineering is required.

References:
===========

  CVE-2007-3846 (http://cve.mitre.org/cgi-bin/cvename.cgi?name=2007-3846)

  http://crisp.cs.du.edu/?q=node/36

Reported by:
============

  Nils Durner and Christian Grothoff, Colorado Research Institute for
  Security and Privacy, http://crisp.cs.du.edu/.

Recommendation:
===============

  Upgrade clients to use Subversion 1.4.5 libraries:

  http://subversion.tigris.org/project_packages.html

Workarounds:
============

  These workarounds apply only to the repository (server) side.  They
  cannot protect a client from a malicious repository administrator.

  * Scan existing repositories for paths containing "\", rename them.

  * Install a pre-commit hook that checks for "\" in filenames.
    Below is such a hook script, indented by four spaces:

    #!/bin/sh
    
    ### backslash-check.py: A Subversion pre-commit hook script to prevent
    ### files containing "\" from being added to the repository.
    ###
    ### See http://cve.mitre.org/cgi-bin/cvename.cgi?name=2007-3846
    
    ###                           *** NOTE: ***
    ### Because Subversion hook scripts execute in a scrubbed environment,
    ### we use an absolute path to the svnlook binary.  You might need to
    ### adjust it for your system.
    SVNLOOK="/usr/bin/svnlook"
    
    ### You shouldn't need to change anything below this line.
    REPOS=${1}
    TXN=${2} 
    
    if ${SVNLOOK} changed -t ${TXN} ${REPOS} | grep -E "^A +.*\\\\"; then
      echo "" >&2
      echo "Cannot commit paths containing '\\':" >&2
      echo "" >&2
      # Show the actual paths:
      ${SVNLOOK} changed -t ${TXN} ${REPOS} \
         | grep -E "^A +.*\\\\" | cut -c5- >&2
      echo "" >&2
      exit 1
    else
      exit 0
    fi

Patch:
======

   This log message and patch applies to Subversion 1.4.4.

[[[
CVE-2007-3846: arbitrary path creation during updates and checkouts.

* subversion/libsvn_wc/update_editor.c
  (check_path_under_root): New helper function.
  (delete_entry, add_or_open_file, open_directory, add_directory):
    Call above, to prevent paths above cwd from being affected.

Patch by: Nils Durner <ndurner@web.de>
          kfogel
]]]

Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c	(revision 26049)
+++ subversion/libsvn_wc/update_editor.c	(working copy)
@@ -793,6 +793,46 @@
   return SVN_NO_ERROR;
 }
       
+
+/* Check that when ADD_PATH is joined to BASE_PATH, the resulting path
+ * is still under BASE_PATH in the local filesystem.  If not, return
+ * SVN_ERR_WC_OBSTRUCTED_UPDATE; else return success.
+ * 
+ * This is to prevent the situation where the repository contains,
+ * say, "..\nastyfile".  Although that's perfectly legal on some
+ * systems, when checked out onto Win32 it would cause "nastyfile" to
+ * be created in the parent of the current edit directory.
+ * 
+ * (http://cve.mitre.org/cgi-bin/cvename.cgi?name=2007-3846)
+ */
+static svn_error_t *
+check_path_under_root(const char *base_path,
+                      const char *add_path,
+                      apr_pool_t *pool)
+{
+  char *newpath;
+  apr_status_t retval;
+
+  retval = apr_filepath_merge
+    (&newpath, base_path, add_path,
+     APR_FILEPATH_NOTABOVEROOT | APR_FILEPATH_SECUREROOTTEST,
+     pool);
+
+  if (retval != APR_SUCCESS)
+    {
+      return svn_error_createf
+        (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
+         _("Path '%s' is not in the working copy"),
+         /* Not using newpath here because it might be NULL or
+            undefined, since apr_filepath_merge() returned error.
+            (Pity we can't pass NULL for &newpath in the first place,
+            but the APR docs don't bless that.) */
+         svn_path_local_style(svn_path_join(base_path, add_path, pool), pool));
+    }
+
+  return SVN_NO_ERROR;
+}
+
 
 /*** The callbacks we'll plug into an svn_delta_editor_t structure. ***/
 
@@ -1033,6 +1073,8 @@
              apr_pool_t *pool)
 {
   struct dir_baton *pb = parent_baton;
+  SVN_ERR(check_path_under_root(pb->path, svn_path_basename(path, pool),
+                                pool));
   return do_entry_deletion(pb->edit_baton, pb->path, path, &pb->log_number,
                            pool);
 }
@@ -1057,6 +1099,8 @@
       || ((! copyfrom_path) && (SVN_IS_VALID_REVNUM(copyfrom_revision))))
     abort();
 
+  SVN_ERR(check_path_under_root(pb->path, db->name, pool));
+
   /* There should be nothing with this name. */
   SVN_ERR(svn_io_check_path(db->path, &kind, db->pool));
   if (kind != svn_node_none)
@@ -1168,6 +1212,8 @@
   struct dir_baton *db = make_dir_baton(path, eb, pb, FALSE, pool);
   *child_baton = db;
 
+  SVN_ERR(check_path_under_root(pb->path, db->name, pool));
+
   /* Mark directory as being at target_revision and URL, but incomplete. */
   tmp_entry.revision = *(eb->target_revision);
   tmp_entry.url = db->new_URL;
@@ -1451,6 +1497,8 @@
 
   fb = make_file_baton(pb, path, adding, pool);
 
+  SVN_ERR(check_path_under_root(fb->dir_baton->path, fb->name, subpool));
+
   /* It is interesting to note: everything below is just validation. We
      aren't actually doing any "work" or fetching any persistent data. */