Merge branch 'stable-4.8' into stable-4.9

* stable-4.8:
  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: Ia7a826399d8d5b8a0eb7169b40e98a6f5c207a4c
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>

6 files changed, 448 insertions, 7 deletions

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 ec19cdc2c2..67fca5e85c 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java
@@ -413,6 +413,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;
@@ -424,6 +425,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;
@@ -700,8 +702,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 19c5c7eb45..e5fbc1a857 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.
 	 *
@@ -684,9 +692,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));
+			}
 		}
 	}
 
@@ -851,10 +865,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]) {
@@ -910,17 +923,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) {
@@ -1027,6 +1054,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;
@@ -1102,4 +1227,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 44abcd598b..72c8664177 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,14 +77,19 @@ 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.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;
@@ -1074,8 +1080,10 @@ public abstract class BaseReceivePack {
 	 */
 	protected void receivePackAndCheckConnectivity() throws IOException {
 		receivePack();
-		if (needCheckConnectivity())
+		if (needCheckConnectivity()) {
+			checkSubmodules();
 			checkConnectivity();
+		}
 		parser = null;
 	}
 
@@ -1393,6 +1401,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;