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";
}
]]]
