Skip to content

Commit

Permalink
Merge pull request #456 from arunvenmany-ibm/contrast_fix
Browse files Browse the repository at this point in the history 
contrast security issue fixes
  • Loading branch information
@cherylking
cherylking authored Sep 27, 2024
2 parents a84fb2d + ff746e1 commit cc1abb6
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 31 deletions.
28 changes: 18 additions & 10 deletions src/main/java/io/openliberty/tools/common/arquillian/util/HttpPortUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand All @@ -36,6 +37,7 @@
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;

import com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.xml.sax.SAXException;
Expand All @@ -46,15 +48,22 @@ public class HttpPortUtil {
public static final int DEFAULT_PORT = 9080;
private static final XPath XPATH = XPathFactory.newInstance().newXPath();

private static final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
private static boolean factoryInitialized = false;
private static DocumentBuilderFactory factory ;

public static void initDocumentBuilderFactory() throws ParserConfigurationException {
if (!factoryInitialized) {
public static DocumentBuilderFactory getBuilderFactory() throws ParserConfigurationException {
if (factory == null) {
factory = DocumentBuilderFactory.newInstance();
factory.setNamespaceAware(true);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
factory.setXIncludeAware(false);
factory.setExpandEntityReferences(false);
}
return factory;
}

public static Integer getHttpPort(File serverXML, File bootstrapProperties)
Expand Down Expand Up @@ -89,8 +98,8 @@ public static Integer getHttpPort(File serverXML, File bootstrapProperties, File

protected static Integer getHttpPortForServerXML(String serverXML, Properties bootstrapProperties, String configVariableXML) throws ParserConfigurationException, SAXException, IOException, XPathExpressionException,
ArquillianConfigurationException {
initDocumentBuilderFactory();
DocumentBuilder builder = factory.newDocumentBuilder();

DocumentBuilder builder = getBuilderFactory().newDocumentBuilder();
Document doc = builder.parse(new ByteArrayInputStream(serverXML.getBytes()));

XPathExpression httpEndpointExpr = XPATH.compile("/server/httpEndpoint");
Expand Down Expand Up @@ -139,9 +148,8 @@ private static String getHttpPortFromConfigVariableXML(String configVariableXML,
if (configVariableXML == null || configVariableXML.length() == 0) {
return null;
}

// get input XML Document
DocumentBuilderFactory inputBuilderFactory = DocumentBuilderFactory.newInstance();
DocumentBuilderFactory inputBuilderFactory = getBuilderFactory();
inputBuilderFactory.setNamespaceAware(false);
inputBuilderFactory.setIgnoringComments(true);
inputBuilderFactory.setCoalescing(true);
inputBuilderFactory.setIgnoringElementContentWhitespace(true);
Expand Down
18 changes: 13 additions & 5 deletions src/main/java/io/openliberty/tools/common/plugins/config/ServerConfigDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.Map;
import java.util.Properties;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand Down Expand Up @@ -71,6 +72,7 @@ public class ServerConfigDocument {
private static final XPathExpression XPATH_SERVER_ENTERPRISE_APPLICATION;
private static final XPathExpression XPATH_SERVER_INCLUDE;
private static final XPathExpression XPATH_SERVER_VARIABLE;
private static final XPathExpression XPATH_ALL_SERVER_APPLICATIONS;

static {
try {
Expand All @@ -80,6 +82,7 @@ public class ServerConfigDocument {
XPATH_SERVER_ENTERPRISE_APPLICATION = xPath.compile("/server/enterpriseApplication");
XPATH_SERVER_INCLUDE = xPath.compile("/server/include");
XPATH_SERVER_VARIABLE = xPath.compile("/server/variable");
XPATH_ALL_SERVER_APPLICATIONS = xPath.compile("/server/application | /server/webApplication | /server/enterpriseApplication");
} catch (XPathExpressionException ex) {
// These XPath expressions should all compile statically.
// Compilation failures mean the expressions are not syntactically
Expand Down Expand Up @@ -141,7 +144,13 @@ private DocumentBuilder getDocumentBuilder() {
docBuilderFactory.setValidating(false);
try {
docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
docBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
docBuilderFactory.setXIncludeAware(false);
docBuilderFactory.setExpandEntityReferences(false);
docBuilder = docBuilderFactory.newDocumentBuilder();
} catch (ParserConfigurationException e) {
// fail catastrophically if we can't create a document builder
Expand Down Expand Up @@ -229,7 +238,7 @@ private void initializeAppsLocation(CommonLoggerI log, File serverXML, File conf
parseApplication(doc, XPATH_SERVER_APPLICATION);
parseApplication(doc, XPATH_SERVER_WEB_APPLICATION);
parseApplication(doc, XPATH_SERVER_ENTERPRISE_APPLICATION);
parseNames(doc, "/server/application | /server/webApplication | /server/enterpriseApplication");
parseNames(doc, XPATH_ALL_SERVER_APPLICATIONS);
parseInclude(doc);
parseConfigDropinsDir();

Expand All @@ -239,10 +248,9 @@ private void initializeAppsLocation(CommonLoggerI log, File serverXML, File conf
}

//Checks for application names in the document. Will add locations without names to a Set
private void parseNames(Document doc, String expression) throws XPathExpressionException, IOException, SAXException {
private void parseNames(Document doc, XPathExpression expression) throws XPathExpressionException, IOException, SAXException {
// parse input document
XPath xPath = XPathFactory.newInstance().newXPath();
NodeList nodeList = (NodeList) xPath.compile(expression).evaluate(doc, XPathConstants.NODESET);
NodeList nodeList = (NodeList) expression.evaluate(doc, XPathConstants.NODESET);

for (int i = 0; i < nodeList.getLength(); i++) {
if (nodeList.item(i).getAttributes().getNamedItem("name") != null) {
Expand Down
32 changes: 26 additions & 6 deletions src/main/java/io/openliberty/tools/common/plugins/config/XmlDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,18 @@
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import javax.xml.transform.OutputKeys;
import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
Expand Down Expand Up @@ -55,9 +59,15 @@ public void createDocument(File xmlFile) throws ParserConfigurationException, SA
builderFactory.setCoalescing(true);
builderFactory.setIgnoringElementContentWhitespace(true);
builderFactory.setValidating(false);
builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
DocumentBuilder builder = builderFactory.newDocumentBuilder();
builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
builderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
builderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
builderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
builderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
builderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
builderFactory.setXIncludeAware(false);
builderFactory.setExpandEntityReferences(false);
DocumentBuilder builder = builderFactory.newDocumentBuilder();
doc = builder.parse(xmlFile);
}

Expand All @@ -70,12 +80,12 @@ public void writeXMLDocument(File f) throws IOException, TransformerException {
if (!f.getParentFile().exists()) {
f.getParentFile().mkdirs();
}
FileOutputStream outFile = new FileOutputStream(f);
OutputStream outFile = Files.newOutputStream(f.toPath());

DOMSource source = new DOMSource(doc);
StreamResult result = new StreamResult(outFile);
StreamResult result = new StreamResult(new OutputStreamWriter(outFile, StandardCharsets.UTF_8));

TransformerFactory transformerFactory = TransformerFactory.newInstance();
TransformerFactory transformerFactory = getTransformerFactory();
Transformer transformer = transformerFactory.newTransformer();
transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "no");
transformer.setOutputProperty(OutputKeys.DOCTYPE_PUBLIC, "yes");
Expand Down Expand Up @@ -109,4 +119,14 @@ public static void addNewlineBeforeFirstElement(File f) throws IOException {
xmlContents = xmlContents.replace("?><", "?>"+System.getProperty("line.separator")+"<");
Files.write(f.toPath(), xmlContents.getBytes());
}

private static TransformerFactory getTransformerFactory() throws TransformerConfigurationException {
TransformerFactory transformerFactory = TransformerFactory.newInstance();
transformerFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, Boolean.TRUE);
// XMLConstants.ACCESS_EXTERNAL_DTD uses an empty string to deny all access to external references;
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
// XMLConstants.ACCESS_EXTERNAL_STYLESHEET uses an empty string to deny all access to external references;
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
return transformerFactory;
}
}
11 changes: 9 additions & 2 deletions src/main/java/io/openliberty/tools/common/plugins/util/DevUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import javax.tools.StandardJavaFileManager;
import javax.tools.StandardLocation;
import javax.tools.ToolProvider;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand Down Expand Up @@ -3537,7 +3538,13 @@ protected Collection<File> getOmitFilesList(File looseAppFile, String srcDirecto
if (looseAppFile != null && looseAppFile.exists()) {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
dbf.setXIncludeAware(false);
dbf.setExpandEntityReferences(false);
DocumentBuilder db = dbf.newDocumentBuilder();
Document document = db.parse(looseAppFile);
NodeList archiveList = document.getElementsByTagName("archive");
Expand Down Expand Up @@ -4608,7 +4615,7 @@ private void untrackContainerfileDirectoriesAndRestart() throws PluginExecutionE
* If container mode, check if any of the files are within a directory specified in one of the Containerfile's
* COPY commands. If not container mode, does nothing.
*
* @param file The files to check, in the same order.
* @param files The files to check, in the same order.
* @return true if container mode and any of the files are within a directory specified in one of the Containerfile's COPY commands.
* @throws IOException if there was an error getting canonical paths
*/
Expand Down
37 changes: 30 additions & 7 deletions src/main/java/io/openliberty/tools/common/plugins/util/PrepareFeatureUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.List;
import java.util.Map;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand Down Expand Up @@ -144,11 +145,8 @@ private Map<File, String> downloadArtifactsFromBOM(File additionalBOM) throws Pl
Map<File, String> result = new HashMap<File, String>();
ArrayList<String> missing_tags = new ArrayList<>();
try {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
DocumentBuilder db = dbf.newDocumentBuilder();
Document doc = db.parse(additionalBOM);
DocumentBuilder db = getDocumentBuilder();
Document doc = db.parse(additionalBOM);
doc.getDocumentElement().normalize();
NodeList dependencyList = doc.getElementsByTagName("dependency");
for (int itr = 0; itr < dependencyList.getLength(); itr++) {
Expand Down Expand Up @@ -182,7 +180,7 @@ private Map<File, String> downloadArtifactsFromBOM(File additionalBOM) throws Pl
result.put(artifactFile, groupId);
}
}
} catch (ParserConfigurationException | SAXException | IOException e) {
} catch (SAXException | IOException e) {
throw new PluginExecutionException("Cannot read the features-bom file " + additionalBOM.getAbsolutePath() + ". " + e.getMessage());

}
Expand Down Expand Up @@ -467,7 +465,32 @@ public void provideJsonFileDependency(File file, String groupId, String version)
*/
public abstract File downloadArtifact(String groupId, String artifactId, String type, String version)
throws PluginExecutionException;


private DocumentBuilder getDocumentBuilder() throws PluginExecutionException {
DocumentBuilder docBuilder;


try {
DocumentBuilderFactory docBuilderFactory = DocumentBuilderFactory.newInstance();
docBuilderFactory.setIgnoringComments(true);
docBuilderFactory.setCoalescing(true);
docBuilderFactory.setIgnoringElementContentWhitespace(true);
docBuilderFactory.setValidating(false);
docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
docBuilderFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
docBuilderFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
docBuilderFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
docBuilderFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
docBuilderFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
docBuilderFactory.setXIncludeAware(false);
docBuilderFactory.setExpandEntityReferences(false);
docBuilder = docBuilderFactory.newDocumentBuilder();
} catch (ParserConfigurationException e) {
// fail catastrophically if we can't create a document builder
throw new PluginExecutionException("Cannot read the features-bom file " + e.getMessage());
}

return docBuilder;
}

}
9 changes: 8 additions & 1 deletion src/main/java/io/openliberty/tools/common/plugins/util/ServerFeatureUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.util.Set;
import java.util.logging.Level;

import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
Expand Down Expand Up @@ -366,7 +367,13 @@ public FeaturesPlatforms getServerXmlFeatures(FeaturesPlatforms origResult, File
try {
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-dtd-grammar", false);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
dbf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
dbf.setXIncludeAware(false);
dbf.setExpandEntityReferences(false);
DocumentBuilder db = dbf.newDocumentBuilder();
db.setErrorHandler(new ErrorHandler() {
@Override
Expand Down

0 comments on commit cc1abb6

Please sign in to comment.