Merge branch 'stable-4.7' into stable-4.8

* stable-4.7:
  Prepare 4.7.6-SNAPSHOT builds
  JGit v4.7.5.201810051826-r
  BaseReceivePack: Validate incoming .gitmodules files
  ObjectChecker: Report .gitmodules files found in the pack
  SubmoduleAddCommand: Reject submodule URIs that look like cli options

Change-Id: Id6fabec4d0b682a7e20a46e88cbc05432efca062
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

11 files changed, 683 insertions, 10 deletions

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java
index 43160fb115..7d298edb8e 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/lib/ObjectCheckerTest.java
@@ -768,6 +768,112 @@ public class ObjectCheckerTest {
 	}
 
 	@Test
+	public void testValidTreeWithGitmodules() throws CorruptObjectException {
+		ObjectId treeId = ObjectId
+				.fromString("0123012301230123012301230123012301230123");
+		StringBuilder b = new StringBuilder();
+		ObjectId blobId = entry(b, "100644 .gitmodules");
+
+		byte[] data = encodeASCII(b.toString());
+		checker.checkTree(treeId, data);
+		assertEquals(1, checker.getGitsubmodules().size());
+		assertEquals(treeId, checker.getGitsubmodules().get(0).getTreeId());
+		assertEquals(blobId, checker.getGitsubmodules().get(0).getBlobId());
+	}
+
+	/*
+	 * Windows case insensitivity and long file name handling
+	 * means that .gitmodules has many synonyms.
+	 *
+	 * Examples inspired by git.git's t/t0060-path-utils.sh, by
+	 * Johannes Schindelin and Congyi Wu.
+	 */
+	@Test
+	public void testNTFSGitmodules() throws CorruptObjectException {
+		for (String gitmodules : new String[] {
+			".GITMODULES",
+			".gitmodules",
+			".Gitmodules",
+			".gitmoduleS",
+			"gitmod~1",
+			"GITMOD~1",
+			"gitmod~4",
+			"GI7EBA~1",
+			"gi7eba~9",
+			"GI7EB~10",
+			"GI7E~123",
+			"~1000000",
+			"~9999999"
+		}) {
+			checker = new ObjectChecker(); // Reset the ObjectChecker state.
+			checker.setSafeForWindows(true);
+			ObjectId treeId = ObjectId
+					.fromString("0123012301230123012301230123012301230123");
+			StringBuilder b = new StringBuilder();
+			ObjectId blobId = entry(b, "100644 " + gitmodules);
+
+			byte[] data = encodeASCII(b.toString());
+			checker.checkTree(treeId, data);
+			assertEquals(1, checker.getGitsubmodules().size());
+			assertEquals(treeId, checker.getGitsubmodules().get(0).getTreeId());
+			assertEquals(blobId, checker.getGitsubmodules().get(0).getBlobId());
+		}
+	}
+
+	@Test
+	public void testNotGitmodules() throws CorruptObjectException {
+		for (String notGitmodules : new String[] {
+			".gitmodu",
+			".gitmodules oh never mind",
+		}) {
+			checker = new ObjectChecker(); // Reset the ObjectChecker state.
+			checker.setSafeForWindows(true);
+			ObjectId treeId = ObjectId
+					.fromString("0123012301230123012301230123012301230123");
+			StringBuilder b = new StringBuilder();
+			entry(b, "100644 " + notGitmodules);
+
+			byte[] data = encodeASCII(b.toString());
+			checker.checkTree(treeId, data);
+			assertEquals(0, checker.getGitsubmodules().size());
+		}
+	}
+
+	/*
+	 * TODO HFS: match ".gitmodules" case-insensitively, after stripping out
+	 * certain zero-length Unicode code points that HFS+ strips out
+	 */
+
+	@Test
+	public void testValidTreeWithGitmodulesUppercase()
+			throws CorruptObjectException {
+		ObjectId treeId = ObjectId
+				.fromString("0123012301230123012301230123012301230123");
+		StringBuilder b = new StringBuilder();
+		ObjectId blobId = entry(b, "100644 .GITMODULES");
+
+		byte[] data = encodeASCII(b.toString());
+		checker.setSafeForWindows(true);
+		checker.checkTree(treeId, data);
+		assertEquals(1, checker.getGitsubmodules().size());
+		assertEquals(treeId, checker.getGitsubmodules().get(0).getTreeId());
+		assertEquals(blobId, checker.getGitsubmodules().get(0).getBlobId());
+	}
+
+	@Test
+	public void testTreeWithInvalidGitmodules() throws CorruptObjectException {
+		ObjectId treeId = ObjectId
+				.fromString("0123012301230123012301230123012301230123");
+		StringBuilder b = new StringBuilder();
+		entry(b, "100644 .gitmodulez");
+
+		byte[] data = encodeASCII(b.toString());
+		checker.checkTree(treeId, data);
+		checker.setSafeForWindows(true);
+		assertEquals(0, checker.getGitsubmodules().size());
+	}
+
+	@Test
 	public void testNullSha1InTreeEntry() throws CorruptObjectException {
 		byte[] data = concat(
 				encodeASCII("100644 A"), new byte[] { '\0' },
@@ -1551,11 +1657,20 @@ public class ObjectCheckerTest {
 		checker.checkTree(encodeASCII(b.toString()));
 	}
 
-	private static void entry(StringBuilder b, final String modeName) {
+	/*
+	 * Returns the id generated for the entry
+	 */
+	private static ObjectId entry(StringBuilder b, String modeName) {
+		byte[] id = new byte[OBJECT_ID_LENGTH];
+
 		b.append(modeName);
 		b.append('\0');
-		for (int i = 0; i < OBJECT_ID_LENGTH; i++)
+		for (int i = 0; i < OBJECT_ID_LENGTH; i++) {
 			b.append((char) i);
+			id[i] = (byte) i;
+		}
+
+		return ObjectId.fromRaw(id);
 	}
 
 	private void assertCorrupt(String msg, int type, StringBuilder b) {
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java
index 5c46659c0a..93f47090a3 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/submodule/SubmoduleAddTest.java
@@ -183,6 +183,37 @@ public class SubmoduleAddTest extends RepositoryTestCase {
 	}
 
 	@Test
+	public void addSubmoduleWithInvalidPath() throws Exception {
+		SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+		command.setPath("-invalid-path");
+		// TODO(ms) set name to a valid value in 5.1.0 and adapt expected
+		// message below
+		command.setURI("http://example.com/repo/x.git");
+		try {
+			command.call().close();
+			fail("Exception not thrown");
+		} catch (IllegalArgumentException e) {
+			// TODO(ms) should check for submodule path, but can't set name
+			// before 5.1.0
+			assertEquals("Invalid submodule name '-invalid-path'",
+					e.getMessage());
+		}
+	}
+
+	@Test
+	public void addSubmoduleWithInvalidUri() throws Exception {
+		SubmoduleAddCommand command = new SubmoduleAddCommand(db);
+		command.setPath("valid-path");
+		command.setURI("-upstream");
+		try {
+			command.call().close();
+			fail("Exception not thrown");
+		} catch (IllegalArgumentException e) {
+			assertEquals("Invalid submodule URL '-upstream'", e.getMessage());
+		}
+	}
+
+	@Test
 	public void addSubmoduleWithRelativeUri() throws Exception {
 		try (Git git = new Git(db)) {
 			writeTrashFile("file.txt", "content");
@@ -269,4 +300,4 @@ public class SubmoduleAddTest extends RepositoryTestCase {
 					ConfigConstants.CONFIG_KEY_URL));
 		}
 	}
-}
\ No newline at end of file
+}
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
index 3411122888..abd2840873 100644
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackAdvertiseRefsHookTest.java
@@ -425,6 +425,71 @@ public class ReceivePackAdvertiseRefsHookTest extends LocalDiskRepositoryTestCas
 	}
 
 	@Test
+	public void testIncludesInvalidGitmodules() throws Exception {
+		final TemporaryBuffer.Heap inBuf = setupSourceRepoInvalidGitmodules();
+		final TemporaryBuffer.Heap outBuf = new TemporaryBuffer.Heap(1024);
+		final ReceivePack rp = new ReceivePack(dst);
+		rp.setCheckReceivedObjects(true);
+		rp.setCheckReferencedObjectsAreReachable(true);
+		rp.setAdvertiseRefsHook(new HidePrivateHook());
+		try {
+			receive(rp, inBuf, outBuf);
+			fail("Expected UnpackException");
+		} catch (UnpackException failed) {
+			Throwable err = failed.getCause();
+			assertTrue(err instanceof IOException);
+		}
+
+		final PacketLineIn r = asPacketLineIn(outBuf);
+		String master = r.readString();
+		int nul = master.indexOf('\0');
+		assertTrue("has capability list", nul > 0);
+		assertEquals(B.name() + ' ' + R_MASTER, master.substring(0, nul));
+		assertSame(PacketLineIn.END, r.readString());
+
+		String errorLine = r.readString();
+		System.out.println(errorLine);
+		assertTrue(errorLine.startsWith(
+				"unpack error Invalid submodule URL '-"));
+		assertEquals("ng refs/heads/s n/a (unpacker error)", r.readString());
+		assertSame(PacketLineIn.END, r.readString());
+	}
+
+	private TemporaryBuffer.Heap setupSourceRepoInvalidGitmodules()
+			throws IOException, Exception, MissingObjectException {
+		String fakeGitmodules = new StringBuilder()
+				.append("[submodule \"test\"]\n")
+				.append("    path = xlib\n")
+				.append("    url = https://example.com/repo/xlib.git\n\n")
+				.append("[submodule \"test2\"]\n")
+				.append("    path = zlib\n")
+				.append("    url = -upayload.sh\n")
+				.toString();
+
+		TestRepository<Repository> s = new TestRepository<>(src);
+		RevBlob blob = s.blob(fakeGitmodules);
+		RevCommit N = s.commit().parent(B)
+				.add(".gitmodules", blob).create();
+		RevTree t = s.parseBody(N).getTree();
+
+		final TemporaryBuffer.Heap pack = new TemporaryBuffer.Heap(1024);
+		packHeader(pack, 3);
+		copy(pack, src.open(N));
+		copy(pack, src.open(t));
+		copy(pack, src.open(blob));
+		digest(pack);
+
+		final TemporaryBuffer.Heap inBuf = new TemporaryBuffer.Heap(1024);
+		final PacketLineOut inPckLine = new PacketLineOut(inBuf);
+		inPckLine.writeString(ObjectId.zeroId().name() + ' ' + N.name() + ' '
+				+ "refs/heads/s" + '\0'
+				+ BasePackPushConnection.CAPABILITY_REPORT_STATUS);
+		inPckLine.end();
+		pack.writeTo(inBuf, PM);
+		return inBuf;
+	}
+
+	@Test
 	public void testUsingUnknownTreeFails() throws Exception {
 		TestRepository<Repository> s = new TestRepository<>(src);
 		RevCommit N = s.commit().parent(B).add("q", s.blob("a")).create();
diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters
index 7c175e3905..abc71b22f3 100644
--- a/org.eclipse.jgit/.settings/.api_filters
+++ b/org.eclipse.jgit/.settings/.api_filters
@@ -32,6 +32,22 @@
             </message_arguments>
         </filter>
     </resource>
+    <resource path="src/org/eclipse/jgit/lib/GitmoduleEntry.java" type="org.eclipse.jgit.lib.GitmoduleEntry">
+        <filter id="1109393411">
+            <message_arguments>
+                <message_argument value="4.7.5"/>
+                <message_argument value="org.eclipse.jgit.lib.GitmoduleEntry"/>
+            </message_arguments>
+        </filter>
+    </resource>
+    <resource path="src/org/eclipse/jgit/lib/ObjectChecker.java" type="org.eclipse.jgit.lib.ObjectChecker">
+        <filter id="1142947843">
+            <message_arguments>
+                <message_argument value="4.7.5"/>
+                <message_argument value="getGitsubmodules()"/>
+            </message_arguments>
+        </filter>
+    </resource>
     <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS">
         <filter id="1141899266">
             <message_arguments>
diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
index c9aaa39945..e0b5bbf800 100644
--- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
+++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties
@@ -347,6 +347,7 @@ invalidDepth=Invalid depth: {0}
 invalidEncryption=Invalid encryption
 invalidExpandWildcard=ExpandFromSource on a refspec that can have mismatched wildcards does not make sense.
 invalidGitdirRef = Invalid .git reference in file ''{0}''
+invalidGitModules=Invalid .gitmodules file
 invalidGitType=invalid git type: {0}
 invalidId=Invalid id: {0}
 invalidId0=Invalid id
@@ -358,6 +359,7 @@ invalidKey=Invalid key: {0}
 invalidLineInConfigFile=Invalid line in config file
 invalidModeFor=Invalid mode {0} for {1} {2} in {3}.
 invalidModeForPath=Invalid mode {0} for path {1}
+invalidNameContainsDotDot=Invalid name (contains ".."): {0}
 invalidObject=Invalid {0} {1}: {2}
 invalidOldIdSent=invalid old id sent
 invalidPacketLineHeader=Invalid packet line header: {0}
@@ -607,7 +609,10 @@ storePushCertMultipleRefs=Store push certificate for {0} refs
 storePushCertOneRef=Store push certificate for {0}
 storePushCertReflog=Store push certificate
 submoduleExists=Submodule ''{0}'' already exists in the index
+submoduleNameInvalid=Invalid submodule name ''{0}''
 submoduleParentRemoteUrlInvalid=Cannot remove segment from remote url ''{0}''
+submodulePathInvalid=Invalid submodule path ''{0}''
+submoduleUrlInvalid=Invalid submodule URL ''{0}''
 submodulesNotSupported=Submodules are not supported
 supportOnlyPackIndexVersion2=Only support index version 2
 symlinkCannotBeWrittenAsTheLinkTarget=Symlink "{0}" cannot be written as the link target cannot be read from within Java.
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java b/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java
index 0519d454ea..e3ba8945df 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/api/SubmoduleAddCommand.java
@@ -51,6 +51,7 @@ import org.eclipse.jgit.api.errors.JGitInternalException;
 import org.eclipse.jgit.api.errors.NoFilepatternException;
 import org.eclipse.jgit.errors.ConfigInvalidException;
 import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.internal.submodule.SubmoduleValidator;
 import org.eclipse.jgit.lib.ConfigConstants;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -158,6 +159,14 @@ public class SubmoduleAddCommand extends
 			throw new IllegalArgumentException(JGitText.get().uriNotConfigured);
 
 		try {
+			SubmoduleValidator.assertValidSubmoduleName(path);
+			SubmoduleValidator.assertValidSubmodulePath(path);
+			SubmoduleValidator.assertValidSubmoduleUri(uri);
+		} catch (SubmoduleValidator.SubmoduleValidationException e) {
+			throw new IllegalArgumentException(e.getMessage());
+		}
+
+		try {
 			if (submoduleExists())
 				throw new JGitInternalException(MessageFormat.format(
 						JGitText.get().submoduleExists, path));
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
index a8dfc2d2a2..aeda4a0496 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -406,6 +406,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidEncryption;
 	/***/ public String invalidExpandWildcard;
 	/***/ public String invalidGitdirRef;
+	/***/ public String invalidGitModules;
 	/***/ public String invalidGitType;
 	/***/ public String invalidId;
 	/***/ public String invalidId0;
@@ -417,6 +418,7 @@ public class JGitText extends TranslationBundle {
 	/***/ public String invalidLineInConfigFile;
 	/***/ public String invalidModeFor;
 	/***/ public String invalidModeForPath;
+	/***/ public String invalidNameContainsDotDot;
 	/***/ public String invalidObject;
 	/***/ public String invalidOldIdSent;
 	/***/ public String invalidPacketLineHeader;
@@ -666,8 +668,11 @@ public class JGitText extends TranslationBundle {
 	/***/ public String storePushCertOneRef;
 	/***/ public String storePushCertReflog;
 	/***/ public String submoduleExists;
-	/***/ public String submodulesNotSupported;
+	/***/ public String submoduleNameInvalid;
 	/***/ public String submoduleParentRemoteUrlInvalid;
+	/***/ public String submodulePathInvalid;
+	/***/ public String submodulesNotSupported;
+	/***/ public String submoduleUrlInvalid;
 	/***/ public String supportOnlyPackIndexVersion2;
 	/***/ public String symlinkCannotBeWrittenAsTheLinkTarget;
 	/***/ public String systemConfigFileInvalid;
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
new file mode 100644
index 0000000000..d4bba2d0d1
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/submodule/SubmoduleValidator.java
@@ -0,0 +1,180 @@
+/*
+ * Copyright (C) 2018, Google LLC.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - 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.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. 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 COPYRIGHT OWNER 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.
+ */
+package org.eclipse.jgit.internal.submodule;
+
+import java.io.IOException;
+import java.text.MessageFormat;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.JGitText;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ConfigConstants;
+
+/**
+ * Validations for the git submodule fields (name, path, uri).
+ *
+ * Invalid values in these fields can cause security problems as reported in
+ * CVE-2018-11235 and and CVE-2018-17456
+ */
+public class SubmoduleValidator {
+
+	/**
+	 * Error validating a git submodule declaration
+	 */
+	public static class SubmoduleValidationException extends Exception {
+
+		/**
+		 * @param message
+		 *            Description of the problem
+		 */
+		public SubmoduleValidationException(String message) {
+			super(message);
+		}
+
+		private static final long serialVersionUID = 1L;
+	}
+
+	/**
+	 * Validate name for a submodule
+	 *
+	 * @param name
+	 *            name of a submodule
+	 * @throws SubmoduleValidationException
+	 *             name doesn't seem valid (detail in message)
+	 */
+	public static void assertValidSubmoduleName(String name)
+			throws SubmoduleValidationException {
+		if (name.contains("/../") || name.contains("\\..\\") //$NON-NLS-1$ //$NON-NLS-2$
+				|| name.startsWith("../") || name.startsWith("..\\") //$NON-NLS-1$ //$NON-NLS-2$
+				|| name.endsWith("/..") || name.endsWith("\\..")) { //$NON-NLS-1$ //$NON-NLS-2$
+			// Submodule names are used to store the submodule repositories
+			// under $GIT_DIR/modules. Having ".." in submodule names makes a
+			// vulnerability (CVE-2018-11235
+			// https://bugs.eclipse.org/bugs/show_bug.cgi?id=535027#c0)
+			// Reject names containing ".." path segments. We don't
+			// automatically replace these characters or canonicalize by
+			// regarding the name as a file path.
+			// Since Path class is platform dependent, we manually check '/' and
+			// '\\' patterns here.
+			throw new SubmoduleValidationException(MessageFormat
+					.format(JGitText.get().invalidNameContainsDotDot, name));
+		}
+
+		if (name.startsWith("-")) { //$NON-NLS-1$
+			throw new SubmoduleValidationException(
+					MessageFormat.format(
+							JGitText.get().submoduleNameInvalid, name));
+		}
+	}
+
+	/**
+	 * Validate URI for a submodule
+	 *
+	 * @param uri
+	 *            uri of a submodule
+	 * @throws SubmoduleValidationException
+	 *             uri doesn't seem valid
+	 */
+	public static void assertValidSubmoduleUri(String uri)
+			throws SubmoduleValidationException {
+		if (uri.startsWith("-")) { //$NON-NLS-1$
+			throw new SubmoduleValidationException(
+					MessageFormat.format(
+							JGitText.get().submoduleUrlInvalid, uri));
+		}
+	}
+
+	/**
+	 * Validate path for a submodule
+	 *
+	 * @param path
+	 *            path of a submodule
+	 * @throws SubmoduleValidationException
+	 *             path doesn't look right
+	 */
+	public static void assertValidSubmodulePath(String path)
+			throws SubmoduleValidationException {
+
+		if (path.startsWith("-")) { //$NON-NLS-1$
+			throw new SubmoduleValidationException(
+					MessageFormat.format(
+							JGitText.get().submodulePathInvalid, path));
+		}
+	}
+
+	/**
+	 * @param gitModulesContents
+	 *            Contents of a .gitmodule file. They will be parsed internally.
+	 * @throws IOException
+	 *             If the contents
+	 */
+	public static void assertValidGitModulesFile(String gitModulesContents)
+			throws IOException {
+		// Validate .gitmodules file
+		Config c = new Config();
+		try {
+			c.fromText(gitModulesContents);
+			for (String subsection : c.getSubsections(
+					ConfigConstants.CONFIG_SUBMODULE_SECTION)) {
+				String url = c.getString(
+						ConfigConstants.CONFIG_SUBMODULE_SECTION,
+						subsection, ConfigConstants.CONFIG_KEY_URL);
+				assertValidSubmoduleUri(url);
+
+				assertValidSubmoduleName(subsection);
+
+				String path = c.getString(
+						ConfigConstants.CONFIG_SUBMODULE_SECTION, subsection,
+						ConfigConstants.CONFIG_KEY_PATH);
+				assertValidSubmodulePath(path);
+			}
+		} catch (ConfigInvalidException e) {
+			throw new IOException(
+					MessageFormat.format(
+							JGitText.get().invalidGitModules,
+							e));
+		} catch (SubmoduleValidationException e) {
+			throw new IOException(e.getMessage(), e);
+		}
+	}
+}
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitmoduleEntry.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitmoduleEntry.java
new file mode 100644
index 0000000000..bded527519
--- /dev/null
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/GitmoduleEntry.java
@@ -0,0 +1,86 @@
+/*
+ * Copyright (C) 2018, Google LLC.
+ * and other copyright owners as documented in the project's IP log.
+ *
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Distribution License v1.0 which
+ * accompanies this distribution, is reproduced below, and is
+ * available at http://www.eclipse.org/org/documents/edl-v10.php
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - 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.
+ *
+ * - Neither the name of the Eclipse Foundation, Inc. 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 COPYRIGHT OWNER 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.
+ */
+package org.eclipse.jgit.lib;
+
+import org.eclipse.jgit.lib.AnyObjectId;
+
+/**
+ * A .gitmodules file found in the pack. Store the blob of the file itself (e.g.
+ * to access its contents) and the tree where it was found (e.g. to check if it
+ * is in the root)
+ *
+ * @since 4.7.5
+ */
+public final class GitmoduleEntry {
+	private final AnyObjectId treeId;
+
+	private final AnyObjectId blobId;
+
+	/**
+	 * A record of (tree, blob) for a .gitmodule file in a pack
+	 *
+	 * @param treeId
+	 *            tree id containing a .gitmodules entry
+	 * @param blobId
+	 *            id of the blob of the .gitmodules file
+	 */
+	public GitmoduleEntry(AnyObjectId treeId, AnyObjectId blobId) {
+		// AnyObjectId's are reused, must keep a copy.
+		this.treeId = treeId.copy();
+		this.blobId = blobId.copy();
+	}
+
+	/**
+	 * @return Id of a .gitmodules file found in the pack
+	 */
+	public AnyObjectId getBlobId() {
+		return blobId;
+	}
+
+	/**
+	 * @return Id of a tree object where the .gitmodules file was found
+	 */
+	public AnyObjectId getTreeId() {
+		return treeId;
+	}
+}
\ No newline at end of file
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
index 9d3aee1508..6ae752c1f5 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/ObjectChecker.java
@@ -44,6 +44,7 @@
 
 package org.eclipse.jgit.lib;
 
+import static org.eclipse.jgit.lib.Constants.DOT_GIT_MODULES;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_LENGTH;
 import static org.eclipse.jgit.lib.Constants.OBJECT_ID_STRING_LENGTH;
 import static org.eclipse.jgit.lib.Constants.OBJ_BAD;
@@ -84,8 +85,10 @@ import static org.eclipse.jgit.util.RawParseUtils.parseBase10;
 
 import java.text.MessageFormat;
 import java.text.Normalizer;
+import java.util.ArrayList;
 import java.util.EnumSet;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Locale;
 import java.util.Set;
 
@@ -136,6 +139,9 @@ public class ObjectChecker {
 	/** Header "tagger " */
 	public static final byte[] tagger = Constants.encodeASCII("tagger "); //$NON-NLS-1$
 
+	/** Path ".gitmodules" */
+	private static final byte[] dotGitmodules = Constants.encodeASCII(DOT_GIT_MODULES);
+
 	/**
 	 * Potential issues identified by the checker.
 	 *
@@ -199,6 +205,8 @@ public class ObjectChecker {
 	private boolean windows;
 	private boolean macosx;
 
+	private final List<GitmoduleEntry> gitsubmodules = new ArrayList<>();
+
 	/**
 	 * Enable accepting specific malformed (but not horribly broken) objects.
 	 *
@@ -678,9 +686,15 @@ public class ObjectChecker {
 				throw new CorruptObjectException(
 						JGitText.get().corruptObjectTruncatedInObjectId);
 			}
+
 			if (ObjectId.zeroId().compareTo(raw, ptr - OBJECT_ID_LENGTH) == 0) {
 				report(NULL_SHA1, id, JGitText.get().corruptObjectZeroId);
 			}
+
+			if (id != null && isGitmodules(raw, lastNameB, lastNameE, id)) {
+				ObjectId blob = ObjectId.fromRaw(raw, ptr - OBJECT_ID_LENGTH);
+				gitsubmodules.add(new GitmoduleEntry(id, blob));
+			}
 		}
 	}
 
@@ -845,10 +859,9 @@ public class ObjectChecker {
 
 	// Mac's HFS+ folds permutations of ".git" and Unicode ignorable characters
 	// to ".git" therefore we should prevent such names
-	private boolean isMacHFSGit(byte[] raw, int ptr, int end,
+	private boolean isMacHFSPath(byte[] raw, int ptr, int end, byte[] path,
 			@Nullable AnyObjectId id) throws CorruptObjectException {
 		boolean ignorable = false;
-		byte[] git = new byte[] { '.', 'g', 'i', 't' };
 		int g = 0;
 		while (ptr < end) {
 			switch (raw[ptr]) {
@@ -904,17 +917,31 @@ public class ObjectChecker {
 				}
 				return false;
 			default:
-				if (g == 4)
+				if (g == path.length) {
 					return false;
-				if (raw[ptr++] != git[g++])
+				}
+				if (toLower(raw[ptr++]) != path[g++]) {
 					return false;
+				}
 			}
 		}
-		if (g == 4 && ignorable)
+		if (g == path.length && ignorable) {
 			return true;
+		}
 		return false;
 	}
 
+	private boolean isMacHFSGit(byte[] raw, int ptr, int end,
+			@Nullable AnyObjectId id) throws CorruptObjectException {
+		byte[] git = new byte[] { '.', 'g', 'i', 't' };
+		return isMacHFSPath(raw, ptr, end, git, id);
+	}
+
+	private boolean isMacHFSGitmodules(byte[] raw, int ptr, int end,
+			@Nullable AnyObjectId id) throws CorruptObjectException {
+		return isMacHFSPath(raw, ptr, end, dotGitmodules, id);
+	}
+
 	private boolean checkTruncatedIgnorableUTF8(byte[] raw, int ptr, int end,
 			@Nullable AnyObjectId id) throws CorruptObjectException {
 		if ((ptr + 2) >= end) {
@@ -1021,6 +1048,104 @@ public class ObjectChecker {
 				&& toLower(buf[p + 2]) == 't';
 	}
 
+	/**
+	 * Check if the filename contained in buf[start:end] could be read as a
+	 * .gitmodules file when checked out to the working directory.
+	 *
+	 * This ought to be a simple comparison, but some filesystems have peculiar
+	 * rules for normalizing filenames:
+	 *
+	 * NTFS has backward-compatibility support for 8.3 synonyms of long file
+	 * names (see
+	 * https://web.archive.org/web/20160318181041/https://usn.pw/blog/gen/2015/06/09/filenames/
+	 * for details). NTFS is also case-insensitive.
+	 *
+	 * MacOS's HFS+ folds away ignorable Unicode characters in addition to case
+	 * folding.
+	 *
+	 * @param buf
+	 *            byte array to decode
+	 * @param start
+	 *            position where a supposed filename is starting
+	 * @param end
+	 *            position where a supposed filename is ending
+	 * @param id
+	 *            object id for error reporting
+	 *
+	 * @return true if the filename in buf could be a ".gitmodules" file
+	 * @throws CorruptObjectException
+	 */
+	private boolean isGitmodules(byte[] buf, int start, int end, @Nullable AnyObjectId id)
+			throws CorruptObjectException {
+		// Simple cases first.
+		if (end - start < 8) {
+			return false;
+		}
+		return (end - start == dotGitmodules.length
+				&& RawParseUtils.match(buf, start, dotGitmodules) != -1)
+			|| (macosx && isMacHFSGitmodules(buf, start, end, id))
+			|| (windows && isNTFSGitmodules(buf, start, end));
+	}
+
+	private boolean matchLowerCase(byte[] b, int ptr, byte[] src) {
+		if (ptr + src.length > b.length) {
+			return false;
+		}
+		for (int i = 0; i < src.length; i++, ptr++) {
+			if (toLower(b[ptr]) != src[i]) {
+				return false;
+			}
+		}
+		return true;
+	}
+
+	// .gitmodules, case-insensitive, or an 8.3 abbreviation of the same.
+	private boolean isNTFSGitmodules(byte[] buf, int start, int end) {
+		if (end - start == 11) {
+			return matchLowerCase(buf, start, dotGitmodules);
+		}
+
+		if (end - start != 8) {
+			return false;
+		}
+
+		// "gitmod" or a prefix of "gi7eba", followed by...
+		byte[] gitmod = new byte[]{'g', 'i', 't', 'm', 'o', 'd', '~'};
+		if (matchLowerCase(buf, start, gitmod)) {
+			start += 6;
+		} else {
+			byte[] gi7eba = new byte[]{'g', 'i', '7', 'e', 'b', 'a'};
+			for (int i = 0; i < gi7eba.length; i++, start++) {
+				byte c = (byte) toLower(buf[start]);
+				if (c == '~') {
+					break;
+				}
+				if (c != gi7eba[i]) {
+					return false;
+				}
+			}
+		}
+
+		// ... ~ and a number
+		if (end - start < 2) {
+			return false;
+		}
+		if (buf[start] != '~') {
+			return false;
+		}
+		start++;
+		if (buf[start] < '1' || buf[start] > '9') {
+			return false;
+		}
+		start++;
+		for (; start != end; start++) {
+			if (buf[start] < '0' || buf[start] > '9') {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	private static boolean isGitTilde1(byte[] buf, int p, int end) {
 		if (end - p != 5)
 			return false;
@@ -1082,4 +1207,17 @@ public class ObjectChecker {
 		String n = RawParseUtils.decode(raw, ptr, end).toLowerCase(Locale.US);
 		return macosx ? Normalizer.normalize(n, Normalizer.Form.NFC) : n;
 	}
+
+	/**
+	 * Get the list of".gitmodules" files found in the pack. For each, report
+	 * its blob id (e.g. to validate its contents) and the tree where it was
+	 * found (e.g. to check if it is in the root)
+	 *
+	 * @return List of pairs of ids <tree, blob>
+	 *
+	 * @since 4.7.5
+	 */
+	public List<GitmoduleEntry> getGitsubmodules() {
+		return gitsubmodules;
+	}
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
index 6f94dbbfec..14b683f017 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java
@@ -43,6 +43,7 @@
 
 package org.eclipse.jgit.transport;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_ATOMIC;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_DELETE_REFS;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_OFS_DELTA;
@@ -76,15 +77,20 @@ import org.eclipse.jgit.errors.PackProtocolException;
 import org.eclipse.jgit.errors.TooLargePackException;
 import org.eclipse.jgit.internal.JGitText;
 import org.eclipse.jgit.internal.storage.file.PackLock;
+import org.eclipse.jgit.internal.submodule.SubmoduleValidator;
+import org.eclipse.jgit.lib.AnyObjectId;
 import org.eclipse.jgit.lib.BatchRefUpdate;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.Config.SectionParser;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.GitmoduleEntry;
 import org.eclipse.jgit.lib.NullProgressMonitor;
 import org.eclipse.jgit.lib.ObjectChecker;
+import org.eclipse.jgit.lib.ObjectDatabase;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectIdSubclassMap;
 import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectLoader;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.ProgressMonitor;
 import org.eclipse.jgit.lib.Ref;
@@ -1081,8 +1087,10 @@ public abstract class BaseReceivePack {
 	 */
 	protected void receivePackAndCheckConnectivity() throws IOException {
 		receivePack();
-		if (needCheckConnectivity())
+		if (needCheckConnectivity()) {
+			checkSubmodules();
 			checkConnectivity();
+		}
 		parser = null;
 	}
 
@@ -1400,6 +1408,21 @@ public abstract class BaseReceivePack {
 				|| !getClientShallowCommits().isEmpty();
 	}
 
+	private void checkSubmodules()
+			throws IOException {
+		ObjectDatabase odb = db.getObjectDatabase();
+		if (objectChecker == null) {
+			return;
+		}
+		for (GitmoduleEntry entry : objectChecker.getGitsubmodules()) {
+			AnyObjectId blobId = entry.getBlobId();
+			ObjectLoader blob = odb.open(blobId, Constants.OBJ_BLOB);
+
+			SubmoduleValidator.assertValidGitModulesFile(
+					new String(blob.getBytes(), UTF_8));
+		}
+	}
+
 	private void checkConnectivity() throws IOException {
 		ObjectIdSubclassMap<ObjectId> baseObjects = null;
 		ObjectIdSubclassMap<ObjectId> providedObjects = null;