[JGIT PATCH v2] Permit a wider range of repository names in jgit daemon requests

From

Shawn O. Pearce <spearce@spearce.org>

Date

Feb 9, 2009, 15:23 UTC

Message-ID

<20090209152337.GH30949@spearce.org>

In-Reply-To

<200902090847.40905.robin.rosenberg@dewire.com>

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
  Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
  > Didn't I tell you // is also UNC-prefix? Windows treats / == \ at the API
  > level. Also why test for contains one "\"? And why in the middle? The
  > UNC prefix can only occur at the beginning of a path. You can use
  > File.isAbsolute to see if a name represents an absolute path. It is
  > platform-depdendent, so new File("//foo/bar").isAbsolute() yield
  > different results on Windows and unix platforms.

  *sigh*... so *that's* what happened to this patch.  I posted it,
  you sent me comments, and I forgot to correct it.  Doh.

  Sorry for blasting you with the same patch twice.  I just really
  forgot that I even posted it, let alone that you had sent me back
  comments and that was why its not in tree yet.

  I think I've addressed your concerns above in this version.
  
 .../src/org/spearce/jgit/transport/Daemon.java     |   56 ++++++++++++--------
 1 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Daemon.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Daemon.java
index 5087533..e5b446c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/Daemon.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Daemon.java
@@ -52,7 +52,6 @@
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.regex.Pattern;
 
 import org.spearce.jgit.lib.PersonIdent;
 import org.spearce.jgit.lib.Repository;
@@ -64,9 +63,6 @@
 
 	private static final int BACKLOG = 5;
 
-	private static final Pattern SAFE_REPOSITORY_NAME = Pattern
-			.compile("^[A-Za-z][A-Za-z0-9/_ -]+(\\.git)?$");
-
 	private InetSocketAddress myAddress;
 
 	private final DaemonService[] services;
@@ -286,8 +282,26 @@ synchronized DaemonService matchService(final String cmd) {
 	}
 
 	Repository openRepository(String name) {
+		// Assume any attempt to use \ was by a Windows client
+		// and correct to the more typical / used in Git URIs.
+		//
+		name = name.replace('\\', '/');
+
+		// git://thishost/path should always be name="/path" here
+		//
 		if (!name.startsWith("/"))
 			return null;
+
+		// Forbid Windows UNC paths as they might escape the base
+		//
+		if (name.startsWith("//"))
+			return null;
+
+		// Forbid funny paths which contain an up-reference, they
+		// might be trying to escape and read /../etc/password.
+		//
+		if (name.contains("/../"))
+			return null;
 		name = name.substring(1);
 
 		Repository db;
@@ -301,24 +315,22 @@ synchronized (exports) {
 				return db;
 		}
 
-		if (SAFE_REPOSITORY_NAME.matcher(name).matches()) {
-			final File[] search;
-			synchronized (exportBase) {
-				search = exportBase.toArray(new File[exportBase.size()]);
-			}
-			for (final File f : search) {
-				db = openRepository(new File(f, name));
-				if (db != null)
-					return db;
-
-				db = openRepository(new File(f, name + ".git"));
-				if (db != null)
-					return db;
-
-				db = openRepository(new File(f, name + "/.git"));
-				if (db != null)
-					return db;
-			}
+		final File[] search;
+		synchronized (exportBase) {
+			search = exportBase.toArray(new File[exportBase.size()]);
+		}
+		for (final File f : search) {
+			db = openRepository(new File(f, name));
+			if (db != null)
+				return db;
+
+			db = openRepository(new File(f, name + ".git"));
+			if (db != null)
+				return db;
+
+			db = openRepository(new File(f, name + "/.git"));
+			if (db != null)
+				return db;
 		}
 		return null;
 	}

-- 
1.6.2.rc0.173.g5e148