fix path traversal vulnerability in git changelog api, see issue #319

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java

@@ -35,6 +35,7 @@ package sonia.scm.repository;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
 
@@ -271,15 +272,25 @@ public class GitUtil
       branchName = PREFIX_HEADS.concat(branchName);
     }
 
-    Ref ref = repo.getRef(branchName);
+    checkBranchName(repo, branchName);
 
-    if (ref != null)
+    try
     {
-      branchId = ref.getObjectId();
+      Ref ref = repo.getRef(branchName);
+
+      if (ref != null)
+      {
+        branchId = ref.getObjectId();
+      }
+      else if (logger.isWarnEnabled())
+      {
+        logger.warn("could not find branch for {}", branchName);
+      }
+
     }
-    else if (logger.isWarnEnabled())
+    catch (Exception ex)
     {
-      logger.warn("could not find branch for {}", branchName);
+      logger.warn("error occured during resolve of branch id", ex);
     }
 
     return branchId;
@@ -453,4 +464,38 @@ public class GitUtil
     return name;
 
   }
+
+  //~--- methods --------------------------------------------------------------
+
+  /**
+   * Method description
+   *
+   *
+   * @param repo
+   * @param branchName
+   *
+   * @throws IOException
+   */
+  @VisibleForTesting
+  static void checkBranchName(org.eclipse.jgit.lib.Repository repo,
+    String branchName)
+    throws IOException
+  {
+    if (branchName.contains(".."))
+    {
+      File repoDirectory = repo.getDirectory();
+      File branchFile = new File(repoDirectory, branchName);
+
+      if (!branchFile.getCanonicalPath().startsWith(
+        repoDirectory.getCanonicalPath()))
+      {
+        logger.error(
+          "branch \"{}\" is outside of the repository. It looks like path traversal attack",
+          branchName);
+
+        throw new IllegalArgumentException(
+          branchName.concat(" is an invalid branch name"));
+      }
+    }
+  }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/GitUtilTest.java

@@ -0,0 +1,100 @@
+/**
+ * Copyright (c) 2010, Sebastian Sdorra All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ * this list of conditions and the following disclaimer. 2. Redistributions in
+ * binary form must reproduce the above copyright notice, this list of
+ * conditions and the following disclaimer in the documentation and/or other
+ * materials provided with the distribution. 3. Neither the name of SCM-Manager;
+ * nor the names of its contributors may be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * http://bitbucket.org/sdorra/scm-manager
+ *
+ */
+
+
+
+package sonia.scm.repository;
+
+//~--- non-JDK imports --------------------------------------------------------
+
+import org.junit.Test;
+
+import static org.mockito.Mockito.*;
+
+//~--- JDK imports ------------------------------------------------------------
+
+import java.io.File;
+import java.io.IOException;
+
+/**
+ *
+ * @author Sebastian Sdorra
+ */
+public class GitUtilTest
+{
+
+  /**
+   * Method description
+   *
+   *
+   * @throws IOException
+   */
+  @Test(expected = IllegalArgumentException.class)
+  public void testCheckInvalidBranchNames() throws IOException
+  {
+    org.eclipse.jgit.lib.Repository repo = mockRepo(new File("/tmp/test"));
+
+    GitUtil.checkBranchName(repo,
+      GitUtil.REF_HEAD_PREFIX.concat("../../../../../etc/passwd"));
+  }
+
+  /**
+   * Method description
+   *
+   *
+   * @throws IOException
+   */
+  @Test
+  public void testCheckValidBranchNames() throws IOException
+  {
+    org.eclipse.jgit.lib.Repository repo = mockRepo(new File("/tmp/test"));
+
+    GitUtil.checkBranchName(repo, GitUtil.REF_HEAD_PREFIX.concat("master"));
+    GitUtil.checkBranchName(repo, GitUtil.REF_HEAD_PREFIX.concat("dev"));
+    GitUtil.checkBranchName(repo, GitUtil.REF_HEAD_PREFIX.concat("develop"));
+  }
+
+  /**
+   * Method description
+   *
+   *
+   * @param directory
+   *
+   * @return
+   */
+  private org.eclipse.jgit.lib.Repository mockRepo(File directory)
+  {
+    org.eclipse.jgit.lib.Repository repo =
+      mock(org.eclipse.jgit.lib.Repository.class);
+
+    when(repo.getDirectory()).thenReturn(directory);
+
+    return repo;
+  }
+}