diff options
author | Brian de Alwis | 2017-08-30 19:47:56 +0000 |
---|---|---|
committer | Brian de Alwis | 2017-08-31 14:03:37 +0000 |
commit | 71ce503c0e1dabec9f7d65841ee4852c59f0432f (patch) | |
tree | 056464f9a7ecd98f8b44b25cfcbf6fe9c9f208cf | |
parent | 70c366fd4d0475ed1fd826cbf2283f8ed3b3ebec (diff) | |
download | rt.equinox.p2-I20170901-2000.tar.gz rt.equinox.p2-I20170901-2000.tar.xz rt.equinox.p2-I20170901-2000.zip |
Bug 518031 - XML External Entity Vulnerability in Eclipse IDEI20170904-0230I20170903-2000I20170902-1500I20170901-2000
Ensure XML processors are configured to use XMLConstants.FEATURE_SECURE_PROCESSING=true
to avoid accessing external DTDs and expanding external entities.
Change-Id: Ic29e4a0aab1ea5f642ce49914bc6fcecd238efe8
Signed-off-by: Brian de Alwis <bsd@mt.ca>
12 files changed, 103 insertions, 24 deletions
diff --git a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorSelector.java b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorSelector.java index 8e7fae428..a8cf92c71 100644 --- a/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorSelector.java +++ b/bundles/org.eclipse.equinox.p2.artifact.repository/src/org/eclipse/equinox/internal/p2/artifact/repository/MirrorSelector.java @@ -24,8 +24,7 @@ import java.util.*; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import org.eclipse.core.runtime.*; -import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; -import org.eclipse.equinox.internal.p2.core.helpers.Tracing; +import org.eclipse.equinox.internal.p2.core.helpers.*; import org.eclipse.equinox.internal.p2.repository.DownloadStatus; import org.eclipse.equinox.internal.p2.repository.Transport; import org.eclipse.equinox.p2.repository.IRepository; @@ -266,7 +265,7 @@ public class MirrorSelector { } mirrorsURL = mirrorsURL + "countryCode=" + countryCode + "&timeZone=" + timeZone + "&format=xml"; //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ - DocumentBuilderFactory domFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory domFactory = SecureXMLUtil.newSecureDocumentBuilderFactory(); DocumentBuilder builder = domFactory.newDocumentBuilder(); Document document = null; // Use Transport to read the mirrors list (to benefit from proxy support, authentication, etc) @@ -292,7 +291,7 @@ public class MirrorSelector { || mirrorsURL.startsWith("https://") //$NON-NLS-1$ || mirrorsURL.startsWith("file://") //$NON-NLS-1$ || mirrorsURL.startsWith("ftp://") //$NON-NLS-1$ - || mirrorsURL.startsWith("jar://"))) //$NON-NLS-1$ + || mirrorsURL.startsWith("jar://"))) //$NON-NLS-1$ log("Error processing mirrors URL: " + mirrorsURL, e); //$NON-NLS-1$ return null; } diff --git a/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/SecureXMLUtil.java b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/SecureXMLUtil.java new file mode 100644 index 000000000..3bd154bc8 --- /dev/null +++ b/bundles/org.eclipse.equinox.p2.core/src/org/eclipse/equinox/internal/p2/core/helpers/SecureXMLUtil.java @@ -0,0 +1,72 @@ +/******************************************************************************* + * Copyright (c) 20017 Manumitting Technologies Inc and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Manumitting Technologies Inc - initial API and implementation + *******************************************************************************/ +package org.eclipse.equinox.internal.p2.core.helpers; + +import javax.xml.XMLConstants; +import javax.xml.parsers.*; +import org.eclipse.core.runtime.IStatus; +import org.eclipse.core.runtime.Status; +import org.eclipse.equinox.internal.p2.core.Activator; +import org.xml.sax.*; +import org.xml.sax.helpers.XMLReaderFactory; + +/** + * A utility class for processing XML data in a secure fashion, + * avoiding XML Entity Expansion problems + */ +public class SecureXMLUtil { + /** + * Create a new {@link DocumentBuilderFactory} suitable for processing + * XML data from possibly malicious sources. For example, data retrieved + * from remote p2 metadata and artifacts repositories. + */ + public static DocumentBuilderFactory newSecureDocumentBuilderFactory() { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + // FEATURE_SECURE_PROCESSING is documented as must be supported by all implementations + try { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (ParserConfigurationException e) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, "Feature not supported", e)); //$NON-NLS-1$ + } + return factory; + } + + /** + * Create a new {@link SAXParserFactory} suitable for processing + * XML data from possibly malicious sources. For example, data retrieved + * from remote p2 metadata and artifacts repositories. + */ + public static SAXParserFactory newSecureSAXParserFactory() { + SAXParserFactory factory = SAXParserFactory.newInstance(); + // FEATURE_SECURE_PROCESSING is documented as must be supported by all implementations + try { + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, "Feature not supported", e)); //$NON-NLS-1$ + } + return factory; + } + + /** + * Create a new {@link XMLReader} suitable for processing + * XML data from possibly malicious sources. For example, data retrieved + * from remote p2 metadata and artifacts repositories. + */ + public static XMLReader newSecureXMLReader() throws SAXException { + XMLReader reader = XMLReaderFactory.createXMLReader(); + try { + reader.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (SAXNotRecognizedException | SAXNotSupportedException e) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, "Feature not supported", e)); //$NON-NLS-1$ + } + return reader; + } +} diff --git a/bundles/org.eclipse.equinox.p2.discovery.compatibility/src/org/eclipse/equinox/internal/p2/discovery/compatibility/DirectoryParser.java b/bundles/org.eclipse.equinox.p2.discovery.compatibility/src/org/eclipse/equinox/internal/p2/discovery/compatibility/DirectoryParser.java index 251b721ad..98ec85241 100644 --- a/bundles/org.eclipse.equinox.p2.discovery.compatibility/src/org/eclipse/equinox/internal/p2/discovery/compatibility/DirectoryParser.java +++ b/bundles/org.eclipse.equinox.p2.discovery.compatibility/src/org/eclipse/equinox/internal/p2/discovery/compatibility/DirectoryParser.java @@ -13,12 +13,12 @@ package org.eclipse.equinox.internal.p2.discovery.compatibility; import java.io.IOException; import java.io.Reader; +import org.eclipse.equinox.internal.p2.core.helpers.SecureXMLUtil; import org.eclipse.equinox.internal.p2.discovery.compatibility.Directory.Entry; import org.eclipse.equinox.internal.p2.discovery.compatibility.util.DefaultSaxErrorHandler; import org.eclipse.equinox.internal.p2.discovery.compatibility.util.IOWithCauseException; import org.eclipse.osgi.util.NLS; import org.xml.sax.*; -import org.xml.sax.helpers.XMLReaderFactory; /** * A parser for {@link Directory directories}. @@ -38,7 +38,7 @@ public class DirectoryParser { public Directory parse(Reader directoryContents) throws IOException { XMLReader xmlReader; try { - xmlReader = XMLReaderFactory.createXMLReader(); + xmlReader = SecureXMLUtil.newSecureXMLReader(); } catch (SAXException e) { throw new IOWithCauseException(e.getMessage(), e); } diff --git a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/io/IUDeserializer.java b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/io/IUDeserializer.java index 07c5baa7b..65d33563d 100644 --- a/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/io/IUDeserializer.java +++ b/bundles/org.eclipse.equinox.p2.metadata.repository/src/org/eclipse/equinox/p2/metadata/io/IUDeserializer.java @@ -15,6 +15,7 @@ import java.io.InputStream; import java.util.Arrays; import java.util.Collection; import javax.xml.parsers.*; +import org.eclipse.equinox.internal.p2.core.helpers.SecureXMLUtil; import org.eclipse.equinox.internal.p2.metadata.repository.io.MetadataParser; import org.eclipse.equinox.internal.p2.persistence.Messages; import org.eclipse.equinox.p2.metadata.IInstallableUnit; @@ -33,7 +34,7 @@ public class IUDeserializer { * Construct a new instance of the deserializer. */ public IUDeserializer() { - deserializer = new IUDeserializerParser(SAXParserFactory.newInstance()); + deserializer = new IUDeserializerParser(SecureXMLUtil.newSecureSAXParserFactory()); } /** diff --git a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/FeatureManifestParser.java b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/FeatureManifestParser.java index a7e0e7281..9e983a9aa 100644 --- a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/FeatureManifestParser.java +++ b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/FeatureManifestParser.java @@ -19,6 +19,7 @@ import java.util.ArrayList; import java.util.List; import javax.xml.parsers.*; import org.eclipse.core.runtime.*; +import org.eclipse.equinox.internal.p2.core.helpers.SecureXMLUtil; import org.eclipse.equinox.p2.metadata.VersionRange; import org.eclipse.equinox.p2.publisher.eclipse.Feature; import org.eclipse.equinox.p2.publisher.eclipse.FeatureEntry; @@ -33,7 +34,7 @@ import org.xml.sax.helpers.DefaultHandler; */ public class FeatureManifestParser extends DefaultHandler { - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory parserFactory = SecureXMLUtil.newSecureSAXParserFactory(); private SAXParser parser; protected Feature result; private URL url; diff --git a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/ProductFile.java b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/ProductFile.java index 1fc305cba..bc632fc1f 100644 --- a/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/ProductFile.java +++ b/bundles/org.eclipse.equinox.p2.publisher.eclipse/src/org/eclipse/equinox/internal/p2/publisher/eclipse/ProductFile.java @@ -25,8 +25,7 @@ import java.util.Map.Entry; import javax.xml.parsers.*; import org.eclipse.core.runtime.*; import org.eclipse.equinox.frameworkadmin.BundleInfo; -import org.eclipse.equinox.internal.p2.core.helpers.ServiceHelper; -import org.eclipse.equinox.internal.p2.core.helpers.URLUtil; +import org.eclipse.equinox.internal.p2.core.helpers.*; import org.eclipse.equinox.p2.metadata.IVersionedId; import org.eclipse.equinox.p2.metadata.VersionedId; import org.eclipse.equinox.p2.publisher.eclipse.FeatureEntry; @@ -67,7 +66,7 @@ public class ProductFile extends DefaultHandler implements IProductDescriptor { private static final String PROPERTY_ECLIPSE_APPLICATION = "eclipse.application"; //$NON-NLS-1$ private static final String PROPERTY_ECLIPSE_PRODUCT = "eclipse.product"; //$NON-NLS-1$ - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory parserFactory = SecureXMLUtil.newSecureSAXParserFactory(); private static final String PROGRAM_ARGS = "programArgs"; //$NON-NLS-1$ private static final String PROGRAM_ARGS_LINUX = "programArgsLin"; //$NON-NLS-1$ diff --git a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/persistence/XMLParser.java b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/persistence/XMLParser.java index fb0570f27..cf03355c2 100644 --- a/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/persistence/XMLParser.java +++ b/bundles/org.eclipse.equinox.p2.repository/src/org/eclipse/equinox/internal/p2/persistence/XMLParser.java @@ -15,8 +15,7 @@ import java.util.*; import javax.xml.parsers.*; import org.eclipse.core.runtime.*; import org.eclipse.equinox.internal.p2.core.Activator; -import org.eclipse.equinox.internal.p2.core.helpers.OrderedProperties; -import org.eclipse.equinox.internal.p2.core.helpers.Tracing; +import org.eclipse.equinox.internal.p2.core.helpers.*; import org.eclipse.equinox.p2.metadata.Version; import org.eclipse.equinox.p2.metadata.VersionRange; import org.eclipse.osgi.util.NLS; @@ -72,7 +71,14 @@ public abstract class XMLParser extends DefaultHandler implements XMLConstants { xmlTracker = new ServiceTracker<SAXParserFactory, SAXParserFactory>(context, SAXParserFactory.class, null); xmlTracker.open(); } - return xmlTracker.getService(); + // FEATURE_SECURE_PROCESSING is documented as must be supported by all implementations + SAXParserFactory factory = xmlTracker.getService(); + try { + factory.setFeature(javax.xml.XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) { + LogHelper.log(new Status(IStatus.WARNING, Activator.ID, "Feature not supported", e)); //$NON-NLS-1$ + } + return factory; } protected synchronized static void releaseXMLParsing() { diff --git a/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/update/ConfigurationParser.java b/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/update/ConfigurationParser.java index da32cc115..d5297eb8f 100644 --- a/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/update/ConfigurationParser.java +++ b/bundles/org.eclipse.equinox.p2.touchpoint.eclipse/src/org/eclipse/equinox/internal/p2/update/ConfigurationParser.java @@ -16,6 +16,7 @@ import java.util.ArrayList; import java.util.StringTokenizer; import javax.xml.parsers.*; import org.eclipse.core.runtime.URIUtil; +import org.eclipse.equinox.internal.p2.core.helpers.SecureXMLUtil; import org.eclipse.equinox.p2.core.ProvisionException; import org.eclipse.osgi.util.NLS; import org.w3c.dom.*; @@ -177,7 +178,7 @@ public class ConfigurationParser implements ConfigurationConstants { */ private Document load(InputStream input) throws ParserConfigurationException, IOException, SAXException { // load the feature xml - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory factory = SecureXMLUtil.newSecureDocumentBuilderFactory(); DocumentBuilder builder = factory.newDocumentBuilder(); input = new BufferedInputStream(input); try { diff --git a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/UpdateManagerCompatibility.java b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/UpdateManagerCompatibility.java index b0173c2cc..192667109 100644 --- a/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/UpdateManagerCompatibility.java +++ b/bundles/org.eclipse.equinox.p2.ui/src/org/eclipse/equinox/internal/p2/ui/UpdateManagerCompatibility.java @@ -17,6 +17,7 @@ import java.util.Iterator; import java.util.Vector; import javax.xml.parsers.*; import org.eclipse.core.runtime.*; +import org.eclipse.equinox.internal.p2.core.helpers.SecureXMLUtil; import org.eclipse.equinox.internal.p2.ui.model.MetadataRepositoryElement; import org.eclipse.equinox.p2.engine.IProvisioningPlan; import org.eclipse.equinox.p2.metadata.IInstallableUnit; @@ -41,7 +42,6 @@ public class UpdateManagerCompatibility { // This value was copied from MetadataGeneratorHelper. Must be the same. private static final String ECLIPSE_INSTALL_HANDLER_PROP = "org.eclipse.update.installHandler"; //$NON-NLS-1$ - private static final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); private static void parse(String fileName, Vector<MetadataRepositoryElement> bookmarks) { File file = new File(fileName); @@ -49,6 +49,7 @@ public class UpdateManagerCompatibility { return; try { + DocumentBuilderFactory documentBuilderFactory = SecureXMLUtil.newSecureDocumentBuilderFactory(); documentBuilderFactory.setNamespaceAware(true); DocumentBuilder parser = documentBuilderFactory.newDocumentBuilder(); Document doc = parser.parse(file); diff --git a/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/CategoryParser.java b/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/CategoryParser.java index f006b40ca..77c10ad12 100644 --- a/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/CategoryParser.java +++ b/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/CategoryParser.java @@ -18,8 +18,7 @@ import java.net.URISyntaxException; import java.util.*; import javax.xml.parsers.*; import org.eclipse.core.runtime.*; -import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; -import org.eclipse.equinox.internal.p2.core.helpers.Tracing; +import org.eclipse.equinox.internal.p2.core.helpers.*; import org.eclipse.equinox.p2.publisher.eclipse.URLEntry; import org.eclipse.equinox.p2.repository.IRepository; import org.eclipse.equinox.p2.repository.spi.RepositoryReference; @@ -32,7 +31,7 @@ import org.xml.sax.helpers.DefaultHandler; * This class was initially copied from org.eclipse.update.core.model.DefaultSiteParser. */ public class CategoryParser extends DefaultHandler { - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory parserFactory = SecureXMLUtil.newSecureSAXParserFactory(); private static final String PLUGIN_ID = Activator.ID; private static final String ARCHIVE = "archive"; //$NON-NLS-1$ diff --git a/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DefaultSiteParser.java b/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DefaultSiteParser.java index 24112bcf2..7570b15c6 100644 --- a/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DefaultSiteParser.java +++ b/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DefaultSiteParser.java @@ -16,8 +16,7 @@ import java.net.URI; import java.util.*; import javax.xml.parsers.*; import org.eclipse.core.runtime.*; -import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; -import org.eclipse.equinox.internal.p2.core.helpers.Tracing; +import org.eclipse.equinox.internal.p2.core.helpers.*; import org.eclipse.equinox.p2.publisher.eclipse.URLEntry; import org.eclipse.osgi.util.NLS; import org.w3c.dom.*; @@ -42,7 +41,7 @@ public class DefaultSiteParser extends DefaultHandler { private static final String BUNDLE = "bundle"; //$NON-NLS-1$ private static final String FEATURES = "features/"; //$NON-NLS-1$ private static final String PLUGINS = "plugins/"; //$NON-NLS-1$ - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory parserFactory = SecureXMLUtil.newSecureSAXParserFactory(); private static final String PLUGIN_ID = Activator.ID; private static final String SITE = "site"; //$NON-NLS-1$ @@ -85,7 +84,7 @@ public class DefaultSiteParser extends DefaultHandler { private static URLEntry[] getAssociateSites(String associateSitesURL) { try { - DocumentBuilderFactory domFactory = DocumentBuilderFactory.newInstance(); + DocumentBuilderFactory domFactory = SecureXMLUtil.newSecureDocumentBuilderFactory(); DocumentBuilder builder = domFactory.newDocumentBuilder(); Document document = builder.parse(associateSitesURL); if (document == null) diff --git a/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DigestParser.java b/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DigestParser.java index 6ce7fd713..068e1a07d 100644 --- a/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DigestParser.java +++ b/bundles/org.eclipse.equinox.p2.updatesite/src/org/eclipse/equinox/internal/p2/updatesite/DigestParser.java @@ -20,6 +20,7 @@ import javax.xml.parsers.*; import org.eclipse.core.runtime.IStatus; import org.eclipse.core.runtime.Status; import org.eclipse.equinox.internal.p2.core.helpers.LogHelper; +import org.eclipse.equinox.internal.p2.core.helpers.SecureXMLUtil; import org.eclipse.equinox.internal.p2.publisher.eclipse.FeatureManifestParser; import org.eclipse.equinox.p2.publisher.eclipse.Feature; import org.eclipse.osgi.util.NLS; @@ -34,7 +35,7 @@ import org.xml.sax.helpers.DefaultHandler; */ public class DigestParser extends DefaultHandler { - private final static SAXParserFactory parserFactory = SAXParserFactory.newInstance(); + private final static SAXParserFactory parserFactory = SecureXMLUtil.newSecureSAXParserFactory(); private SAXParser parser; private final List<Feature> features = new ArrayList<>(); private final FeatureManifestParser featureHandler = new FeatureManifestParser(false); |