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 7ea504374..531c918e9 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 @@ -836,6 +836,112 @@ public class ObjectCheckerTest { checker.checkTree(encodeASCII(b.toString())); } + @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( @@ -1607,11 +1713,20 @@ public class ObjectCheckerTest { checker.checkTree(encodeASCII(b.toString())); } - private static void entry(StringBuilder b, 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 0676eab2d..1ff64a2e2 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 @@ -225,6 +225,34 @@ public class SubmoduleAddTest extends RepositoryTestCase { } } + @Test + public void addSubmoduleWithInvalidPath() throws Exception { + SubmoduleAddCommand command = new SubmoduleAddCommand(db); + command.setPath("-invalid-path"); + command.setName("sub"); + command.setURI("http://example.com/repo/x.git"); + try { + command.call().close(); + fail("Exception not thrown"); + } catch (IllegalArgumentException e) { + assertEquals("Invalid submodule path '-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)) { 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 9cc6cfd39..c959f6c49 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 @@ -468,6 +468,71 @@ public class ReceivePackAdvertiseRefsHookTest extends LocalDiskRepositoryTestCas assertSame(PacketLineIn.END, r.readString()); } + @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 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 s = new TestRepository<>(src); diff --git a/org.eclipse.jgit/.settings/.api_filters b/org.eclipse.jgit/.settings/.api_filters index 9e56703c5..282d7e896 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -3,32 +3,24 @@ + - - - - - - - - - + + - - - + + - - + + - - - + + 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 58f451917..3f1d21289 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -367,6 +367,7 @@ invalidEncryption=Invalid encryption invalidExpandWildcard=ExpandFromSource on a refspec that can have mismatched wildcards does not make sense. invalidFilter=Invalid filter: {0} invalidGitdirRef = Invalid .git reference in file ''{0}'' +invalidGitModules=Invalid .gitmodules file invalidGitType=invalid git type: {0} invalidId=Invalid id: {0} invalidId0=Invalid id @@ -379,7 +380,7 @@ invalidLineInConfigFile=Invalid line in config file invalidLineInConfigFileWithParam=Invalid line in config file: {0} invalidModeFor=Invalid mode {0} for {1} {2} in {3}. invalidModeForPath=Invalid mode {0} for path {1} -invalidNameContainsDotDot=Invalid name (contains ".."): {1} +invalidNameContainsDotDot=Invalid name (contains ".."): {0} invalidObject=Invalid {0} {1}: {2} invalidOldIdSent=invalid old id sent invalidPacketLineHeader=Invalid packet line header: {0} @@ -663,7 +664,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 b8d4468a2..244a15686 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; @@ -194,6 +195,14 @@ public class SubmoduleAddCommand extends .format(JGitText.get().invalidNameContainsDotDot, name)); } + try { + SubmoduleValidator.assertValidSubmoduleName(name); + SubmoduleValidator.assertValidSubmodulePath(path); + SubmoduleValidator.assertValidSubmoduleUri(uri); + } catch (SubmoduleValidator.SubmoduleValidationException e) { + throw new IllegalArgumentException(e.getMessage()); + } + try { if (submoduleExists()) throw new JGitInternalException(MessageFormat.format( 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 96f428a9a..c11ae5a52 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -428,6 +428,7 @@ public class JGitText extends TranslationBundle { /***/ public String invalidExpandWildcard; /***/ public String invalidFilter; /***/ public String invalidGitdirRef; + /***/ public String invalidGitModules; /***/ public String invalidGitType; /***/ public String invalidId; /***/ public String invalidId0; @@ -724,8 +725,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 000000000..d4bba2d0d --- /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 000000000..bded52751 --- /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 689659b39..59e2834cb 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 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)); + } } } @@ -861,10 +875,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]) { @@ -920,17 +933,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) { @@ -1037,6 +1064,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; @@ -1113,4 +1238,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 + * + * @since 4.7.5 + */ + public List 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 c426e3c05..d3419bc20 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; @@ -1185,8 +1191,10 @@ public abstract class BaseReceivePack { */ protected void receivePackAndCheckConnectivity() throws IOException { receivePack(); - if (needCheckConnectivity()) + if (needCheckConnectivity()) { + checkSubmodules(); checkConnectivity(); + } parser = null; } @@ -1510,6 +1518,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 baseObjects = null; ObjectIdSubclassMap providedObjects = null; diff --git a/pom.xml b/pom.xml index 44d151a29..26bd7d188 100644 --- a/pom.xml +++ b/pom.xml @@ -475,7 +475,7 @@ org.apache.maven.plugins maven-javadoc-plugin - -Xdoclint:-missing + -Xdoclint:-missing ${project.build.sourceEncoding} true org.eclipse.jgit.http.test @@ -554,7 +554,7 @@ - -Xdoclint:none + -Xdoclint:-missing