Config: Remove the include functionality
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.
This was fixed on master [1] by making "readIncludedConfig" a noop
by default. This allows only FileBasedConfig, which originated from
local disk, to read local system paths.
However, the "readIncludedConfig" method was only introduced in [2]
which was needed by [3], both of which are only on the master branch.
On the stable branch only Config supports includes. Therefore this
commit simply disables the include functionality.
[1] https://git.eclipse.org/r/#/c/113371/
[2] https://git.eclipse.org/r/#/c/111847/
[3] https://git.eclipse.org/r/#/c/111848/
Bug: 528781
Change-Id: I9a3be3f1d07c4b6772bff535a2556e699a61381c
Signed-off-by: David Pursehouse <david.pursehouse@gmail.com>
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 e9505f6..a12831a 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
@@ -80,6 +80,7 @@
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.SystemReader;
import org.junit.After;
+import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
@@ -766,6 +767,7 @@ public void testReadMultipleValuesForName() throws ConfigInvalidException {
}
@Test
+ @Ignore
public void testIncludeInvalidName() throws ConfigInvalidException {
expectedEx.expect(ConfigInvalidException.class);
expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile);
@@ -773,6 +775,7 @@ public void testIncludeInvalidName() throws ConfigInvalidException {
}
@Test
+ @Ignore
public void testIncludeNoValue() throws ConfigInvalidException {
expectedEx.expect(ConfigInvalidException.class);
expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile);
@@ -780,6 +783,7 @@ public void testIncludeNoValue() throws ConfigInvalidException {
}
@Test
+ @Ignore
public void testIncludeEmptyValue() throws ConfigInvalidException {
expectedEx.expect(ConfigInvalidException.class);
expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile);
@@ -816,6 +820,7 @@ public void testIncludeValuePathRelative() throws ConfigInvalidException {
}
@Test
+ @Ignore
public void testIncludeTooManyRecursions() throws IOException {
File config = tmp.newFile("config");
String include = "[include]\npath=" + config.toPath() + "\n";
@@ -832,27 +837,14 @@ 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=" + more.toPath() + "\n";
- String includeOther = "path=" + other.toPath() + "\n";
- String fooPlus = fooBar + includeMore + includeOther;
- 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());
+ Files.write(config.toPath(), fooBar.getBytes());
Config parsed = parse("[include]\npath=" + config.toPath() + "\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 6281bcf..b0f5c2c 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;
@@ -70,8 +67,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;
/**
* Git style {@code .config}, {@code .gitconfig}, {@code .gitmodules} file.
@@ -1073,10 +1068,6 @@ private List<ConfigLine> fromTextRecurse(final String text, int depth)
e.value = MAGIC_EMPTY_VALUE;
} else
e.value = readValue(in, false, -1);
-
- if (e.section.equals("include")) { //$NON-NLS-1$
- addIncludedConfig(newEntries, e, depth);
- }
} else
throw new ConfigInvalidException(JGitText.get().invalidLineInConfigFile);
}
@@ -1084,36 +1075,6 @@ private List<ConfigLine> fromTextRecurse(final String text, int depth)
return newEntries;
}
- private void addIncludedConfig(final List<ConfigLine> newEntries,
- ConfigLine line, int depth) throws ConfigInvalidException {
- if (!line.name.equals("path") || //$NON-NLS-1$
- line.value == null || line.value.equals(MAGIC_EMPTY_VALUE)) {
- throw new ConfigInvalidException(
- JGitText.get().invalidLineInConfigFile);
- }
- File path = new File(line.value);
- try {
- byte[] bytes = IO.readFully(path);
- String decoded;
- if (isUtf8(bytes)) {
- decoded = RawParseUtils.decode(RawParseUtils.UTF8_CHARSET,
- bytes, 3, bytes.length);
- } else {
- decoded = RawParseUtils.decode(bytes);
- }
- newEntries.addAll(fromTextRecurse(decoded, depth + 1));
- } catch (FileNotFoundException fnfe) {
- if (path.exists()) {
- throw new ConfigInvalidException(MessageFormat
- .format(JGitText.get().cannotReadFile, path), fnfe);
- }
- } catch (IOException ioe) {
- throw new ConfigInvalidException(
- MessageFormat.format(JGitText.get().cannotReadFile, path),
- ioe);
- }
- }
-
private ConfigSnapshot newState() {
return new ConfigSnapshot(Collections.<ConfigLine> emptyList(),
getBaseState());
