diff options
| -rw-r--r-- | org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java | 33 | ||||
| -rw-r--r-- | org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java | 12 |
2 files changed, 44 insertions, 1 deletions
diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java index fca27d32aa..0949d040e9 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/gitrepo/ManifestParserTest.java @@ -12,12 +12,16 @@ package org.eclipse.jgit.gitrepo; import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; import java.net.URI; +import java.nio.file.Files; +import java.nio.file.StandardOpenOption; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -221,4 +225,33 @@ public class ManifestParserTest { testNormalize("", ""); testNormalize("a/b", "a/b"); } + + @Test + public void testXXE() throws Exception { + File externalEntity = File.createTempFile("injected", "xml"); + externalEntity.deleteOnExit(); + Files.write(externalEntity.toPath(), + "<evil>injected xml</evil>" + .getBytes(UTF_8), + StandardOpenOption.WRITE); + String baseUrl = "https://git.google.com/"; + StringBuilder xmlContent = new StringBuilder(); + xmlContent.append("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n") + .append("<!DOCTYPE booo [ <!ENTITY foobar SYSTEM \"") + .append(externalEntity.getPath()).append("\"> ]>\n") + .append("<manifest>") + .append("<remote name=\"remote1\" fetch=\".\" />") + .append("<default revision=\"master\" remote=\"remote1\" />") + .append("&foobar;") + .append("<project path=\"foo\" name=\"foo\" groups=\"a,test\" />") + .append("</manifest>"); + + IOException e = assertThrows(IOException.class, + () -> new ManifestParser(null, null, "master", baseUrl, null, + null) + .read(new ByteArrayInputStream( + xmlContent.toString().getBytes(UTF_8)))); + assertTrue(e.getCause().getMessage().contains("DOCTYPE")); + } + } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java index b033177e05..58b4d3dc56 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/gitrepo/ManifestParser.java @@ -142,7 +142,17 @@ public class ManifestParser extends DefaultHandler { xmlInRead++; final XMLReader xr; try { - xr = SAXParserFactory.newInstance().newSAXParser().getXMLReader(); + SAXParserFactory spf = SAXParserFactory.newInstance(); + spf.setFeature( + "http://xml.org/sax/features/external-general-entities", //$NON-NLS-1$ + false); + spf.setFeature( + "http://xml.org/sax/features/external-parameter-entities", //$NON-NLS-1$ + false); + spf.setFeature( + "http://apache.org/xml/features/disallow-doctype-decl", //$NON-NLS-1$ + true); + xr = spf.newSAXParser().getXMLReader(); } catch (SAXException | ParserConfigurationException e) { throw new IOException(JGitText.get().noXMLParserAvailable, e); } |