Skip to content
Permalink
Browse files Browse the repository at this point in the history
Fix XXE vulnerabilities in multiple add-ons
Signed-off-by: Kai Kreuzer <kai@openhab.org>
  • Loading branch information
kaikreuzer committed Jan 27, 2021
1 parent c57d3ee commit 81935b0
Show file tree
Hide file tree
Showing 28 changed files with 163 additions and 13 deletions.
Expand Up @@ -18,6 +18,8 @@

import javax.xml.bind.JAXBException;
import javax.xml.bind.Unmarshaller;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.smarthome.core.thing.ThingStatus;
Expand Down Expand Up @@ -62,15 +64,16 @@ public void execute(int status, String response) {
logger.trace("Received State response {}", response);
if (isValidRequest()) {
try {
XMLStreamReader xsr = JAXBUtils.XMLINPUTFACTORY.createXMLStreamReader(new StringReader(response));
Unmarshaller unmarshaller = JAXBUtils.JAXBCONTEXT_DEVICES.createUnmarshaller();
DeviceListModel model = (DeviceListModel) unmarshaller.unmarshal(new StringReader(response));
DeviceListModel model = (DeviceListModel) unmarshaller.unmarshal(xsr);
if (model != null) {
handler.onDeviceListAdded(model.getDevicelist());
} else {
logger.debug("no model in response");
}
handler.setStatusInfo(ThingStatus.ONLINE, ThingStatusDetail.NONE, null);
} catch (JAXBException e) {
} catch (JAXBException | XMLStreamException e) {
logger.error("Exception creating Unmarshaller: {}", e.getLocalizedMessage(), e);
handler.setStatusInfo(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
e.getLocalizedMessage());
Expand Down
Expand Up @@ -18,6 +18,8 @@

import javax.xml.bind.JAXBException;
import javax.xml.bind.Unmarshaller;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.binding.avmfritz.internal.dto.templates.TemplateListModel;
Expand Down Expand Up @@ -58,14 +60,15 @@ public void execute(int status, String response) {
logger.trace("Received response '{}'", response);
if (isValidRequest()) {
try {
XMLStreamReader xsr = JAXBUtils.XMLINPUTFACTORY.createXMLStreamReader(new StringReader(response));
Unmarshaller unmarshaller = JAXBUtils.JAXBCONTEXT_TEMPLATES.createUnmarshaller();
TemplateListModel model = (TemplateListModel) unmarshaller.unmarshal(new StringReader(response));
TemplateListModel model = (TemplateListModel) unmarshaller.unmarshal(xsr);
if (model != null) {
handler.addTemplateList(model.getTemplates());
} else {
logger.debug("no template in response");
}
} catch (JAXBException e) {
} catch (JAXBException | XMLStreamException e) {
logger.error("Exception creating Unmarshaller: {}", e.getLocalizedMessage(), e);
}
} else {
Expand Down
Expand Up @@ -14,6 +14,7 @@

import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
import javax.xml.stream.XMLInputFactory;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand All @@ -34,6 +35,7 @@ public class JAXBUtils {

public static final @Nullable JAXBContext JAXBCONTEXT_DEVICES = initJAXBContextDevices();
public static final @Nullable JAXBContext JAXBCONTEXT_TEMPLATES = initJAXBContextTemplates();
public static final XMLInputFactory XMLINPUTFACTORY = initXMLInputFactory();

private static @Nullable JAXBContext initJAXBContextDevices() {
try {
Expand All @@ -52,4 +54,11 @@ public class JAXBUtils {
return null;
}
}

private static XMLInputFactory initXMLInputFactory() {
XMLInputFactory xif = XMLInputFactory.newInstance();
xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
return xif;
}
}
Expand Up @@ -41,6 +41,7 @@ public XMLResponseProcessor(BoseSoundTouchHandler handler) {

public void handleMessage(String msg) throws SAXException, IOException {
XMLReader reader = XMLReaderFactory.createXMLReader();
reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
reader.setContentHandler(new XMLResponseHandler(handler, stateSwitchingMap));
reader.parse(new InputSource(new StringReader(msg)));
}
Expand Down
Expand Up @@ -309,6 +309,8 @@ private <T> T getDocument(String uri, Class<T> response) throws IOException {
if (StringUtils.isNotBlank(result)) {
JAXBContext jc = JAXBContext.newInstance(response);
XMLInputFactory xif = XMLInputFactory.newInstance();
xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
XMLStreamReader xsr = xif.createXMLStreamReader(IOUtils.toInputStream(result));
xsr = new PropertyRenamerDelegate(xsr);

Expand Down
Expand Up @@ -261,8 +261,15 @@ private void autoConfigure() {

if (status == HttpURLConnection.HTTP_OK && response != null) {
DocumentBuilderFactory domFactory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder;
try {
// see
// https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
domFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
domFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
domFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
domFactory.setXIncludeAware(false);
domFactory.setExpandEntityReferences(false);
DocumentBuilder builder;
builder = domFactory.newDocumentBuilder();
Document dDoc = builder.parse(new InputSource(new StringReader(response.getContentAsString())));
XPath xPath = XPathFactory.newInstance().newXPath();
Expand Down
Expand Up @@ -155,7 +155,14 @@ public DLinkHNAPCommunication(final String ipAddress, final String pin) {
uri = new URI("http://" + ipAddress + "/HNAP1");
httpClient.start();

parser = DocumentBuilderFactory.newInstance().newDocumentBuilder();
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setXIncludeAware(false);
dbf.setExpandEntityReferences(false);
parser = dbf.newDocumentBuilder();

final MessageFactory messageFactory = MessageFactory.newInstance();
requestAction = messageFactory.createMessage();
Expand Down
Expand Up @@ -82,8 +82,18 @@ public class Enigma2Client {
private final DocumentBuilderFactory factory;

public Enigma2Client(String host, @Nullable String user, @Nullable String password, int requestTimeout) {
this.enigma2HttpClient = new Enigma2HttpClient(requestTimeout);
this.factory = DocumentBuilderFactory.newInstance();
enigma2HttpClient = new Enigma2HttpClient(requestTimeout);
factory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
try {
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
} catch (ParserConfigurationException e) {
logger.warn("Failed setting parser features against XXE attacks!", e);
}
if (StringUtils.isNotEmpty(user) && StringUtils.isNotEmpty(password)) {
this.host = "http://" + user + ":" + password + "@" + host;
} else {
Expand Down
Expand Up @@ -105,6 +105,12 @@ public String getNamespaceURI(@Nullable String prefix) {
public Client() {
documentBuilderFactory.setNamespaceAware(true);
try {
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
documentBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
documentBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
documentBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
documentBuilderFactory.setXIncludeAware(false);
documentBuilderFactory.setExpandEntityReferences(false);
documentBuilder = documentBuilderFactory.newDocumentBuilder();
} catch (ParserConfigurationException e) {
throw new IllegalStateException(e);
Expand Down
Expand Up @@ -209,6 +209,12 @@ public String getSessionId() {
private Document getXmlDocFromString(String xmlString)
throws ParserConfigurationException, SAXException, IOException {
final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
final DocumentBuilder builder = factory.newDocumentBuilder();
final Document xmlDocument = builder.parse(new InputSource(new StringReader(xmlString)));
return xmlDocument;
Expand Down
Expand Up @@ -61,7 +61,14 @@ public StatusFileInterpreter(String hostname, Ipx800EventListener listener) {

public void read() {
try {
DocumentBuilder builder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
DocumentBuilder builder = factory.newDocumentBuilder();
String statusPage = HttpUtil.executeUrl("GET", String.format(URL_TEMPLATE, hostname), 5000);
InputStream inputStream = new ByteArrayInputStream(statusPage.getBytes());
Document document = builder.parse(inputStream);
Expand Down
Expand Up @@ -47,6 +47,9 @@ public XmlRpcResponse(InputStream is, String encoding)
throws SAXException, ParserConfigurationException, IOException {
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParser saxParser = factory.newSAXParser();
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
saxParser.getXMLReader().setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
InputSource inputSource = new InputSource(is);
inputSource.setEncoding(encoding);
saxParser.parse(inputSource, new XmlRpcHandler());
Expand Down
Expand Up @@ -50,7 +50,7 @@ public class HPWebServerClient {

/**
* Creates a new HP Web Server Client object.
*
*
* @param httpClient {HttpClient} The HttpClient to use for HTTP requests.
* @param address The address for the Embedded Web Server.
*/
Expand All @@ -63,7 +63,7 @@ public HPWebServerClient(HttpClient httpClient, String address) {

/**
* Gets the Status information from the Embedded Web Server.
*
*
* @return The status information.
*/
public HPServerResult<HPStatus> getStatus() {
Expand All @@ -84,7 +84,7 @@ public HPServerResult<HPScannerStatusFeatures> getScannerFeatures() {

/**
* Gets the Usage information from the Embedded Web Server.
*
*
* @return The usage information.
*/
public HPServerResult<HPUsage> getUsage() {
Expand Down Expand Up @@ -120,6 +120,12 @@ private <T> HPServerResult<T> fetchData(String endpoint, Function<Document, T> f

private synchronized Document getDocument(String contentAsString)
throws ParserConfigurationException, SAXException, IOException {
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
DocumentBuilder builder = factory.newDocumentBuilder();
InputSource source = new InputSource(new StringReader(contentAsString));
return builder.parse(source);
Expand Down
Expand Up @@ -53,6 +53,12 @@ public static Document readFromFile(String filePath) throws IhcExecption {
File fXmlFile = new File(filePath);
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
try {
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
dbFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbFactory.setXIncludeAware(false);
dbFactory.setExpandEntityReferences(false);
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
Document doc = dBuilder.parse(fXmlFile);
return doc;
Expand Down
Expand Up @@ -78,6 +78,12 @@ public HashMap<String, DeviceType> getDeviceTypes() {
*/
public void loadDeviceTypesXML(InputStream in) throws ParserConfigurationException, SAXException, IOException {
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
dbFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbFactory.setXIncludeAware(false);
dbFactory.setExpandEntityReferences(false);
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
Document doc = dBuilder.parse(in);
doc.getDocumentElement().normalize();
Expand Down
Expand Up @@ -52,6 +52,12 @@ public static List<FeatureTemplate> readTemplates(InputStream input) throws IOEx
List<FeatureTemplate> features = new ArrayList<>();
try {
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
dbFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbFactory.setXIncludeAware(false);
dbFactory.setExpandEntityReferences(false);
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
// Parse it!
Document doc = dBuilder.parse(input);
Expand Down
Expand Up @@ -56,6 +56,12 @@ public static HashMap<String, Msg> readMessageDefinitions(InputStream input)
HashMap<String, Msg> messageMap = new HashMap<>();
try {
DocumentBuilderFactory dbFactory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
dbFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbFactory.setXIncludeAware(false);
dbFactory.setExpandEntityReferences(false);
DocumentBuilder dBuilder = dbFactory.newDocumentBuilder();
// Parse it!
Document doc = dBuilder.parse(input);
Expand Down
Expand Up @@ -498,6 +498,12 @@ public void statusUpdateReceived(String ip, EiscpMessage data) {
private void processInfo(String infoXML) {
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
DocumentBuilder builder = factory.newDocumentBuilder();
try (StringReader sr = new StringReader(infoXML)) {
InputSource is = new InputSource(sr);
Expand Down
Expand Up @@ -81,6 +81,12 @@ public static HashMap<String, String> buildHashMap(String... data) {
public static @Nullable Document loadXMLFromString(String xml) {
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
DocumentBuilder builder = factory.newDocumentBuilder();
InputSource is = new InputSource(new StringReader(xml));
return builder.parse(is);
Expand Down
Expand Up @@ -134,6 +134,7 @@ public static List<SonosEntry> getEntriesFromString(String xml) {
*/
public static @Nullable SonosResourceMetaData getResourceMetaData(String xml) throws SAXException {
XMLReader reader = XMLReaderFactory.createXMLReader();
reader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
ResourceMetaDataHandler handler = new ResourceMetaDataHandler();
reader.setContentHandler(handler);
try {
Expand Down

0 comments on commit 81935b0

Please sign in to comment.