diff options
author | Matthias Sohn <matthias.sohn@sap.com> | 2018-10-06 16:56:12 +0200 |
---|---|---|
committer | Matthias Sohn <matthias.sohn@sap.com> | 2018-10-06 16:56:12 +0200 |
commit | dff99a272a96777a445cf4e822ffc22067222ce9 (patch) | |
tree | 06349d3a7eff369372dd15e41f9575805faa8da0 | |
parent | 9dbb18291fc680ec9326b866f133b233093d800f (diff) | |
parent | 23ebbe5662f10a4b8ad47b1fb1cfe115e03d2ddd (diff) | |
download | jgit-dff99a272a96777a445cf4e822ffc22067222ce9.tar.gz jgit-dff99a272a96777a445cf4e822ffc22067222ce9.zip |
Merge branch 'stable-5.0' into stable-5.1
* stable-5.0:
Prepare 4.11.5-SNAPSHOT builds
JGit v4.11.4.201810060650-r
Fix configuration of maven-javadoc-plugin
Prepare 4.9.7-SNAPSHOT builds
JGit v4.9.6.201810051924-r
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
* Fix todos in SubmoduleAddTest
Change-Id: I53272081094b8948a40a1ce409af08b6ef330c1e
Signed-off-by: Jonathan Nieder <jrn@google.com>
Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
12 files changed, 673 insertions, 29 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 7ea504374c..531c918e9b 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 @@ -837,6 +837,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' }, @@ -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 0676eab2d6..1ff64a2e28 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 @@ -226,6 +226,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)) { writeTrashFile("file.txt", "content"); 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 9cc6cfd393..c959f6c497 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 @@ -469,6 +469,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 9e56703c57..282d7e8962 100644 --- a/org.eclipse.jgit/.settings/.api_filters +++ b/org.eclipse.jgit/.settings/.api_filters @@ -3,32 +3,24 @@ <resource path="META-INF/MANIFEST.MF"> <filter id="924844039"> <message_arguments> + <message_argument value="5.1.2"/> <message_argument value="5.1.0"/> - <message_argument value="5.1.0"/> - </message_arguments> - </filter> - <filter id="924844039"> - <message_arguments> - <message_argument value="5.0.4"/> - <message_argument value="5.0.0"/> </message_arguments> </filter> </resource> - <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS"> - <filter id="1141899266"> + <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"/> - <message_argument value="5.1"/> - <message_argument value="createNewFileAtomic(File)"/> + <message_argument value="4.7.5"/> + <message_argument value="org.eclipse.jgit.lib.GitmoduleEntry"/> </message_arguments> </filter> </resource> - <resource path="src/org/eclipse/jgit/util/FS.java" type="org.eclipse.jgit.util.FS$LockToken"> - <filter id="1141899266"> + <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"/> - <message_argument value="5.1"/> - <message_argument value="LockToken"/> + <message_argument value="4.7.5"/> + <message_argument value="getGitsubmodules()"/> </message_arguments> </filter> </resource> 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 58f4519171..3f1d21289f 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 b8d4468a2c..244a15686f 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; @@ -195,6 +196,14 @@ public class SubmoduleAddCommand extends } 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( 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 96f428a9af..c11ae5a526 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 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 689659b398..59e2834cb8 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)); + } } } @@ -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 <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 c426e3c05d..d3419bc201 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<ObjectId> baseObjects = null; ObjectIdSubclassMap<ObjectId> providedObjects = null; @@ -475,7 +475,7 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-javadoc-plugin</artifactId> <configuration> - <additionalparam>-Xdoclint:-missing</additionalparam> + <additionalJOption>-Xdoclint:-missing</additionalJOption> <encoding>${project.build.sourceEncoding}</encoding> <quiet>true</quiet> <excludePackageNames>org.eclipse.jgit.http.test</excludePackageNames> @@ -554,7 +554,7 @@ </reportSet> </reportSets> <configuration> - <additionalparam>-Xdoclint:none</additionalparam> + <additionalJOption>-Xdoclint:-missing</additionalJOption> </configuration> </plugin> <plugin> |