From 692ecba7fd463e2b5f438059f123764a07fbdf49 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Thu, 18 Dec 2014 10:33:02 +0100 Subject: [PATCH 1/1] BUG-2511 Fix XXE vulnerability in initial config loaders Change-Id: I0b9aefa8d7a2d59f542075172fe03678618a6385 Signed-off-by: Maros Marsalek --- .../directory/xml/XmlDirectoryPersister.java | 15 +++++++++++++-- .../internal/FeatureConfigSnapshotHolder.java | 16 ++++++++++++++-- .../persist/storage/file/xml/model/Config.java | 13 ++++++++++--- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/opendaylight/config/config-persister-directory-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/directory/xml/XmlDirectoryPersister.java b/opendaylight/config/config-persister-directory-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/directory/xml/XmlDirectoryPersister.java index 85f70b9a01..3ea432e173 100644 --- a/opendaylight/config/config-persister-directory-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/directory/xml/XmlDirectoryPersister.java +++ b/opendaylight/config/config-persister-directory-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/directory/xml/XmlDirectoryPersister.java @@ -23,6 +23,10 @@ import java.util.SortedSet; import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBException; import javax.xml.bind.Unmarshaller; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import javax.xml.transform.stream.StreamSource; import org.opendaylight.controller.config.persist.api.ConfigSnapshotHolder; import org.opendaylight.controller.config.persist.api.Persister; import org.opendaylight.controller.config.persist.storage.file.xml.model.ConfigSnapshot; @@ -105,8 +109,15 @@ public class XmlDirectoryPersister implements Persister { public static ConfigSnapshotHolder loadLastConfig(final File file) throws JAXBException { JAXBContext jaxbContext = JAXBContext.newInstance(ConfigSnapshot.class); Unmarshaller um = jaxbContext.createUnmarshaller(); - - return asHolder((ConfigSnapshot) um.unmarshal(file)); + XMLInputFactory xif = XMLInputFactory.newFactory(); + xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + xif.setProperty(XMLInputFactory.SUPPORT_DTD, false); + try { + XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(file)); + return asHolder((ConfigSnapshot) um.unmarshal(xsr)); + } catch (final XMLStreamException e) { + throw new JAXBException(e); + } } private static ConfigSnapshotHolder asHolder(final ConfigSnapshot unmarshalled) { diff --git a/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigSnapshotHolder.java b/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigSnapshotHolder.java index 1bce5f2364..518716cfa7 100644 --- a/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigSnapshotHolder.java +++ b/opendaylight/config/config-persister-feature-adapter/src/main/java/org/opendaylight/controller/configpusherfeature/internal/FeatureConfigSnapshotHolder.java @@ -20,6 +20,10 @@ import java.util.SortedSet; import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBException; import javax.xml.bind.Unmarshaller; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import javax.xml.transform.stream.StreamSource; import org.apache.karaf.features.ConfigFileInfo; import org.apache.karaf.features.Feature; import org.opendaylight.controller.config.persist.api.ConfigSnapshotHolder; @@ -59,10 +63,18 @@ public class FeatureConfigSnapshotHolder implements ConfigSnapshotHolder { Preconditions.checkNotNull(feature); this.fileInfo = fileInfo; this.featureChain.add(feature); + // TODO extract utility method for umarshalling config snapshots JAXBContext jaxbContext = JAXBContext.newInstance(ConfigSnapshot.class); Unmarshaller um = jaxbContext.createUnmarshaller(); - File file = new File(fileInfo.getFinalname()); - unmarshalled = ((ConfigSnapshot) um.unmarshal(file)); + XMLInputFactory xif = XMLInputFactory.newFactory(); + xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + xif.setProperty(XMLInputFactory.SUPPORT_DTD, false); + try { + XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(new File(fileInfo.getFinalname()))); + unmarshalled = ((ConfigSnapshot) um.unmarshal(xsr)); + } catch (final XMLStreamException e) { + throw new JAXBException(e); + } } /* * (non-Javadoc) diff --git a/opendaylight/config/config-persister-file-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/file/xml/model/Config.java b/opendaylight/config/config-persister-file-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/file/xml/model/Config.java index e629d20db5..6a6d360cfa 100644 --- a/opendaylight/config/config-persister-file-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/file/xml/model/Config.java +++ b/opendaylight/config/config-persister-file-xml-adapter/src/main/java/org/opendaylight/controller/config/persist/storage/file/xml/model/Config.java @@ -22,6 +22,10 @@ import javax.xml.bind.Unmarshaller; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlElementWrapper; import javax.xml.bind.annotation.XmlRootElement; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; +import javax.xml.stream.XMLStreamReader; +import javax.xml.transform.stream.StreamSource; import org.apache.commons.lang3.StringUtils; @XmlRootElement(name = "persisted-snapshots") @@ -72,9 +76,12 @@ public final class Config { try { JAXBContext jaxbContext = JAXBContext.newInstance(Config.class); Unmarshaller um = jaxbContext.createUnmarshaller(); - - return (Config) um.unmarshal(from); - } catch (JAXBException e) { + XMLInputFactory xif = XMLInputFactory.newFactory(); + xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + xif.setProperty(XMLInputFactory.SUPPORT_DTD, false); + XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(from)); + return ((Config) um.unmarshal(xsr)); + } catch (JAXBException | XMLStreamException e) { throw new PersistException("Unable to restore configuration", e); } } -- 2.36.6