Subversion releases up to 1.6.22 (inclusive), and 1.7.x tags up to 1.7.10
   (inclusive, but excepting 1.7.x releases made from those tags), 
   include a contrib/ script prone to shell injection by authenticated users,
   which could result in arbitrary code execution.

Summary:
========

Subversion's contrib/ directory contains two example hook scripts, which
use 'svnlook changed' to examine a revision or transaction and then pass
those paths as arguments to further 'svnlook' commands, without properly
escaping the command-line.

The contrib/ directory ships in 1.6.x releases, and although it does not
ship in 1.7.x or 1.8.x releases, is included in the 1.7.x and 1.8.x
release branches and tags in Subversion's repository.

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

  Subversion releases through 1.6.22 (inculsive)
  Repository revisions branches/1.7.x until r1485487
  Repository revisions branches/1.8.x until r1485487
  Subversion tags through 1.7.10 (inclusive)

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

  Releases:
  Subversion 1.6.23
  Subversion 1.7.0
  Subversion 1.8.0

  Tags:
  Subversion 1.6.23
  Subversion 1.7.11
  Subversion 1.8.0-rc3
  Subversion 1.8.0

Details:
========

  The script contrib/hook-scripts/check-mime-type.pl does not escape
  argv arguments to 'svnlook' that start with a hyphen.  This could be
  used to cause 'svnlook', and hence check-mime-type.pl, to error out.

  The script contrib/hook-scripts/svn-keyword-check.pl parses filenames
  from the output of 'svnlook changed' and passes them to a further
  shell command (equivalent to the 'system()' call of the C standard
  library) without escaping them.  This could be used to run arbitrary
  shell commands in the context of the user whom the pre-commit script
  runs as (the user who owns the repository).

Severity:
=========

  CVSSv2 Base Score: 7.1
  CVSSv2 Base Vector: AV:N/AC:H/Au:S/C:C/I:C/A:C  

  Most installations of Subversion do not use these contrib scripts, so
  while the score above is high, we suspect that very few sites are impacted.
  However, if you do use these scripts, this is a serious issue.

  The check-mime-type.pl issue could only be a problem if 'svnlook' was
  patched or if a child of the repository root had a name starting with
  a '-', so it is ranked as low severity.

  The svn-keyword-check.pl issue could be used by any authenticated
  committer to run shell commands as the server.  Anonymous users
  typically do not have commit access so cannot exploit this.  On the
  other hand, those who can exploit this could, for example, delete
  the repository from the server disk.

Recommendations:
================

  We recommend all users to apply the attached patch.  The hook scripts
  have not changed since 1.6.x, so using their latest versions from the
  repository is (as of this writing) equivalent to applying the patch.

  The fix will be included in the 1.6.23, 1.7.11, and 1.8.0 releases,
  when those are made.

  A workaround is to ensure that all in-repository filenames are shell-safe,
  e.g., match the regular expression
      ^[A-Za-z0-9_:][A-Za-z0-9_:/-]+$
  .  This can be implemented using the provided [validate-files.py] hook
  script, by providing a command= that checks the environment variable "FILE"
  against that pattern; for example, command= might point to the following
  script:

      #!/usr/bin/env python
      import os, re, sys
      re = r'^[A-Za-z0-9_:][A-Za-z0-9_:/-]+$'
      sys.exit(re.compile(re).match(os.getenv("FILE", " ")))

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

  CVE-2013-2088  (Subversion)

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

Daniel Shahaf, elego Software Solutions GmbH

Patches:
========

  Patch against 1.6.21, 1.7.x branch/tags, and 1.8.x branch:
[[[
Index: contrib/hook-scripts/check-mime-type.pl
===================================================================
--- contrib/hook-scripts/check-mime-type.pl	(revision 1484585)
+++ contrib/hook-scripts/check-mime-type.pl	(working copy)
@@ -120,7 +120,7 @@ foreach my $path ( @files_added )
 		# Parse the complete list of property values of the file $path to extract
 		# the mime-type and eol-style
 		foreach my $prop (&read_from_process($svnlook, 'proplist', $repos, '-t',
-		                  $txn, '--verbose', $path))
+		                  $txn, '--verbose', '--', $path))
 			{
 				if ($prop =~ /^\s*svn:mime-type : (\S+)/)
 					{
@@ -187,7 +187,7 @@ sub safe_read_from_pipe
       croak "$0: safe_read_from_pipe passed no arguments.\n";
     }
   print "Running @_\n";
-  my $pid = open(SAFE_READ, '-|');
+  my $pid = open(SAFE_READ, '-|', @_);
   unless (defined $pid)
     {
       die "$0: cannot fork: $!\n";
Index: contrib/hook-scripts/svn-keyword-check.pl
===================================================================
--- contrib/hook-scripts/svn-keyword-check.pl	(revision 1484585)
+++ contrib/hook-scripts/svn-keyword-check.pl	(working copy)
@@ -141,7 +141,7 @@ sub check {
                 return 1;
             } else {
                 my @keywords = get_svnkeywords($file);
-                my $fh = _pipe("$svnlook cat $flag $value $repos $file");
+                my $fh = _pipe($svnlook, qw/cat/, $flag, $value, $repos, '--', $file);
                 while (my $line = <$fh>) {
                     foreach my $keyword (@keywords) {
                         if ($line =~ m/$keyword/) {
@@ -168,7 +168,7 @@ sub file_is_binary {
         return 0;
     }
     if (has_svn_property($file, "svn:mime-type")) {
-        my ($mimetype) = read_from_process("$svnlook propget $flag $value $repos svn:mime-type $file");
+        my ($mimetype) = read_from_process($svnlook, qw/propget/, $flag, $value, $repos, 'svn:mime-type', '--', $file);
         chomp($mimetype);
         $mimetype =~ s/^\s*(.*)/$1/;
         if ($mimetype =~ m/^text\//) {
@@ -186,7 +186,7 @@ sub file_is_binary {
 # Return a list of svn:keywords on a file
 sub get_svnkeywords {
     my $file = shift;
-    my @lines = read_from_process("$svnlook propget $flag $value $repos svn:keywords $file");
+    my @lines = read_from_process($svnlook, qw/propget/, $flag, $value, $repos, 'svn:keywords', '--', $file);
     my @returnlines;
     foreach my $line (@lines) {
         $line =~ s/\s+/ /;
@@ -199,7 +199,7 @@ sub get_svnkeywords {
 sub has_svn_property {
     my $file = shift;
     my $keyword = shift;
-    my @proplist = read_from_process("$svnlook proplist $flag $value $repos $file");
+    my @proplist = read_from_process($svnlook, qw/proplist/, $flag, $value, $repos, '--', $file);
     foreach my $prop (@proplist) {
         chomp($prop);
         if ($prop =~ m/\b$keyword\b/) {
@@ -241,7 +241,7 @@ sub safe_read_from_pipe {
 # Return the filehandle as a glob so we can loop over it elsewhere.
 sub _pipe {
     local *SAFE_READ;
-    my $pid = open(SAFE_READ, '-|');
+    my $pid = open(SAFE_READ, '-|', @_);
     unless (defined $pid) {
         die "$0: cannot fork: $!\n";
     }
]]]