From ec7f88eec8f52a2254b82bcb73aa489028ea3b39 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 15 Dec 2017 08:50:28 +0900 Subject: 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 --- .../tst/org/eclipse/jgit/lib/ConfigTest.java | 24 ++++++++-------------- 1 file changed, 8 insertions(+), 16 deletions(-) (limited to 'org.eclipse.jgit.test/tst/org/eclipse') 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 e9505f67d0..a12831a149 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.storage.file.FileBasedConfig; 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 class ConfigTest { } @Test + @Ignore public void testIncludeInvalidName() throws ConfigInvalidException { expectedEx.expect(ConfigInvalidException.class); expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile); @@ -773,6 +775,7 @@ public class ConfigTest { } @Test + @Ignore public void testIncludeNoValue() throws ConfigInvalidException { expectedEx.expect(ConfigInvalidException.class); expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile); @@ -780,6 +783,7 @@ public class ConfigTest { } @Test + @Ignore public void testIncludeEmptyValue() throws ConfigInvalidException { expectedEx.expect(ConfigInvalidException.class); expectedEx.expectMessage(JGitText.get().invalidLineInConfigFile); @@ -816,6 +820,7 @@ public class ConfigTest { } @Test + @Ignore public void testIncludeTooManyRecursions() throws IOException { File config = tmp.newFile("config"); String include = "[include]\npath=" + config.toPath() + "\n"; @@ -832,27 +837,14 @@ public class ConfigTest { } @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 { -- cgit v1.2.3