From 3a7704638abf5d221a05509291e7c49ad1ac63ba Mon Sep 17 00:00:00 2001
From: Shawn Pearce <spearce@spearce.org>
Date: Wed, 13 Dec 2017 17:35:38 -0800
Subject: [PATCH] Make Config.readIncludedConfig a noop by default

The Config class must be safe to run against untrusted input files.
Reading arbitrary local system paths using include.path is risky for
servers, including Gerrit Code Review.  Return null by default to
incide the include should be ignored.

Only FileBasedConfig which originated from local disk should be trying
to read local system paths.  FileBasedConfig already overrides this
method with its own implementation.

Change-Id: I2ff31753868aa1bbac4a6843a4c23e50bd6f46f3
---
 .../tst/org/eclipse/jgit/lib/ConfigTest.java  | 18 +++---------------
 .../src/org/eclipse/jgit/lib/Config.java      | 19 +------------------
 2 files changed, 4 insertions(+), 33 deletions(-)

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
index 3f4478a77..3deb7a60f 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
@@ -833,27 +833,15 @@ public void testIncludeTooManyRecursions() throws IOException {
 	}
 
 	@Test
-	public void testInclude() throws IOException, ConfigInvalidException {
+	public void testIncludeIsNoop() throws IOException, ConfigInvalidException {
 		File config = tmp.newFile("config");
-		File more = tmp.newFile("config.more");
-		File other = tmp.newFile("config.other");
 
 		String fooBar = "[foo]\nbar=true\n";
-		String includeMore = "[include]\npath=" + pathToString(more) + "\n";
-		String includeOther = "path=" + pathToString(other) + "\n";
-		String fooPlus = fooBar + includeMore + includeOther;
+		String fooPlus = fooBar;
 		Files.write(config.toPath(), fooPlus.getBytes());
 
-		String fooMore = "[foo]\nmore=bar\n";
-		Files.write(more.toPath(), fooMore.getBytes());
-
-		String otherMore = "[other]\nmore=bar\n";
-		Files.write(other.toPath(), otherMore.getBytes());
-
 		Config parsed = parse("[include]\npath=" + pathToString(config) + "\n");
-		assertTrue(parsed.getBoolean("foo", "bar", false));
-		assertEquals("bar", parsed.getString("foo", null, "more"));
-		assertEquals("bar", parsed.getString("other", null, "more"));
+		assertFalse(parsed.getBoolean("foo", "bar", false));
 	}
 
 	private static void assertReadLong(long exp) throws ConfigInvalidException {
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
index f655f062b..3e28184b5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java
@@ -51,9 +51,6 @@
 
 package org.eclipse.jgit.lib;
 
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.IOException;
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -71,7 +68,6 @@
 import org.eclipse.jgit.events.ListenerList;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.transport.RefSpec;
-import org.eclipse.jgit.util.IO;
 import org.eclipse.jgit.util.RawParseUtils;
 
 /**
@@ -1115,20 +1111,7 @@ private List<ConfigLine> fromTextRecurse(final String text, int depth)
 	@Nullable
 	protected byte[] readIncludedConfig(String relPath)
 			throws ConfigInvalidException {
-		File path = new File(relPath);
-		try {
-			return IO.readFully(path);
-		} catch (FileNotFoundException fnfe) {
-			if (path.exists()) {
-				throw new ConfigInvalidException(MessageFormat
-						.format(JGitText.get().cannotReadFile, path), fnfe);
-			}
-			return null;
-		} catch (IOException ioe) {
-			throw new ConfigInvalidException(
-					MessageFormat.format(JGitText.get().cannotReadFile, path),
-					ioe);
-		}
+		return null;
 	}
 
 	private void addIncludedConfig(final List<ConfigLine> newEntries,