Browse Source

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
stable-4.10
Shawn Pearce 7 years ago
parent
commit
3a7704638a
  1. 18
      org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java
  2. 17
      org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java

18
org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ConfigTest.java

@ -833,27 +833,15 @@ public class ConfigTest {
} }
@Test @Test
public void testInclude() throws IOException, ConfigInvalidException { public void testIncludeIsNoop() throws IOException, ConfigInvalidException {
File config = tmp.newFile("config"); File config = tmp.newFile("config");
File more = tmp.newFile("config.more");
File other = tmp.newFile("config.other");
String fooBar = "[foo]\nbar=true\n"; String fooBar = "[foo]\nbar=true\n";
String includeMore = "[include]\npath=" + pathToString(more) + "\n"; String fooPlus = fooBar;
String includeOther = "path=" + pathToString(other) + "\n";
String fooPlus = fooBar + includeMore + includeOther;
Files.write(config.toPath(), fooPlus.getBytes()); 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"); Config parsed = parse("[include]\npath=" + pathToString(config) + "\n");
assertTrue(parsed.getBoolean("foo", "bar", false)); assertFalse(parsed.getBoolean("foo", "bar", false));
assertEquals("bar", parsed.getString("foo", null, "more"));
assertEquals("bar", parsed.getString("other", null, "more"));
} }
private static void assertReadLong(long exp) throws ConfigInvalidException { private static void assertReadLong(long exp) throws ConfigInvalidException {

17
org.eclipse.jgit/src/org/eclipse/jgit/lib/Config.java

@ -51,9 +51,6 @@
package org.eclipse.jgit.lib; package org.eclipse.jgit.lib;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.text.MessageFormat; import java.text.MessageFormat;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
@ -71,7 +68,6 @@ import org.eclipse.jgit.events.ListenerHandle;
import org.eclipse.jgit.events.ListenerList; import org.eclipse.jgit.events.ListenerList;
import org.eclipse.jgit.internal.JGitText; import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.transport.RefSpec; import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.util.IO;
import org.eclipse.jgit.util.RawParseUtils; import org.eclipse.jgit.util.RawParseUtils;
/** /**
@ -1115,20 +1111,7 @@ public class Config {
@Nullable @Nullable
protected byte[] readIncludedConfig(String relPath) protected byte[] readIncludedConfig(String relPath)
throws ConfigInvalidException { 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; return null;
} catch (IOException ioe) {
throw new ConfigInvalidException(
MessageFormat.format(JGitText.get().cannotReadFile, path),
ioe);
}
} }
private void addIncludedConfig(final List<ConfigLine> newEntries, private void addIncludedConfig(final List<ConfigLine> newEntries,

Loading…
Cancel
Save