Skip to content

Commit

Permalink
Bug 577894 - Security Issue -- XXE Attack
Browse files Browse the repository at this point in the history
Applications using XMLMemento are vulnerable to XXE Attack

see https://docs.oracle.com/en/java/javase/17/security/java-api-xml-processing-jaxp-security-guide.html

Change-Id: I31013372fe98566731410406dcad3044dc6992d9
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/188792
Reviewed-by: Kalyan Prasad Tatavarthi <kalyan_prasad@in.ibm.com>
Tested-by: Platform Bot <platform-bot@eclipse.org>
(cherry picked from commit 0e1a84f)
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/189017
Tested-by: Sarika Sinha <sarika.sinha@in.ibm.com>
Reviewed-by: Sarika Sinha <sarika.sinha@in.ibm.com>
  • Loading branch information
SarikaSinha committed Jan 4, 2022
1 parent 4785157 commit 6a6523e
Showing 1 changed file with 45 additions and 3 deletions.
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2018 IBM Corporation and others.
* Copyright (c) 2000, 2021 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -19,6 +19,7 @@
import java.io.StringWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.Arrays;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand Down Expand Up @@ -51,6 +52,8 @@ public final class XMLMemento implements IMemento {

private Element element;

private static String FILE_STRING = "file"; //$NON-NLS-1$

/**
* Creates a <code>Document</code> from the <code>Reader</code> and returns a
* memento on the first <code>Element</code> for reading the document.
Expand All @@ -66,6 +69,28 @@ public static XMLMemento createReadRoot(Reader reader) throws WorkbenchException
return createReadRoot(reader, null);
}

/**
* Clients who need to use the "file" protocol can override this method to
* return the original attribute value
*
* @param attributeOldValue
* @return return the new attribute value after concatenating the "file"
* protocol restriction if does not exist already
*/
private static String getAttributeNewValue(Object attributeOldValue) {
StringBuffer strNewValue = new StringBuffer(FILE_STRING);
if (attributeOldValue instanceof String && ((String) attributeOldValue).length() != 0) {
String strOldValue = (String) attributeOldValue;
boolean exists = Arrays.asList(strOldValue.split(",")).stream().anyMatch(x -> x.trim().equals(FILE_STRING)); //$NON-NLS-1$
if (!exists) {
strNewValue.append(", ").append(strOldValue); //$NON-NLS-1$
} else {
strNewValue = new StringBuffer(strOldValue);
}
}
return strNewValue.toString();
}

/**
* Creates a <code>Document</code> from the <code>Reader</code> and returns a
* memento on the first <code>Element</code> for reading the document.
Expand All @@ -82,9 +107,21 @@ public static XMLMemento createReadRoot(Reader reader) throws WorkbenchException
public static XMLMemento createReadRoot(Reader reader, String baseDir) throws WorkbenchException {
String errorMessage = null;
Exception exception = null;

DocumentBuilderFactory factory = null;
Object attributeDTDOldValue = null;
Object attributeSchemaOldValue = null;
try {
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
factory = DocumentBuilderFactory.newInstance();
try {
attributeDTDOldValue = factory.getAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD);
attributeSchemaOldValue = factory.getAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_SCHEMA);
} catch (NullPointerException | IllegalArgumentException e) {
// Attributes not defined
}
factory.setAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD,
getAttributeNewValue(attributeDTDOldValue));
factory.setAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_SCHEMA,
getAttributeNewValue(attributeSchemaOldValue));
DocumentBuilder parser = factory.newDocumentBuilder();
InputSource source = new InputSource(reader);
if (baseDir != null) {
Expand Down Expand Up @@ -125,6 +162,11 @@ public void fatalError(SAXParseException exception) throws SAXException {
} catch (SAXException e) {
exception = e;
errorMessage = WorkbenchMessages.XMLMemento_formatError;
} finally {
if (factory != null) {
factory.setAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_DTD, attributeDTDOldValue);
factory.setAttribute(javax.xml.XMLConstants.ACCESS_EXTERNAL_SCHEMA, attributeSchemaOldValue);
}
}

String problemText = null;
Expand Down

0 comments on commit 6a6523e

Please sign in to comment.