Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(4.x.x) Add options for securing the XML Parser #2147

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions conf.xml.tmpl
Expand Up @@ -566,6 +566,18 @@
<!--
Default settings for parsing structured documents:

- xml (optional)

- features
Any default SAX2 feature flags to set on the parser

- feature
- name
the name of the feature flag
- value
the value of the feature flag


- html-to-xml (optional)

- class
Expand Down Expand Up @@ -603,6 +615,22 @@
-->
<parser>

<xml>

<features>

<!-- NOTE: the following feature flags should likely be set in production to ensure a secure environment -->

<!--
<feature name="http://xml.org/sax/features/external-general-entities" value="false"/>
<feature name="http://xml.org/sax/features/external-parameter-entities" value="false"/>
<feature name="http://javax.xml.XMLConstants/feature/secure-processing" value="true"/>
-->

</features>

</xml>

<!-- html-to-xml class="org.ccil.cowan.tagsoup.Parser"/ -->

<html-to-xml class="org.cyberneko.html.parsers.SAXParser">
Expand Down
22 changes: 22 additions & 0 deletions src/org/exist/util/Configuration.java
Expand Up @@ -538,6 +538,28 @@ private void configureTransformer( Element transformer )
}

private void configureParser(final Element parser) {
configureXmlParser(parser);
configureHtmlToXmlParser(parser);
}

private void configureXmlParser(final Element parser) {
final NodeList nlXml = parser.getElementsByTagName(XMLReaderPool.XmlParser.XML_PARSER_ELEMENT);
if(nlXml.getLength() > 0) {
final Element xml = (Element)nlXml.item(0);

final NodeList nlFeatures = xml.getElementsByTagName(XMLReaderPool.XmlParser.XML_PARSER_FEATURES_ELEMENT);
if(nlFeatures.getLength() > 0) {
final Properties pFeatures = ParametersExtractor.parseFeatures(nlFeatures.item(0));
if(pFeatures != null) {
final Map<String, Boolean> features = new HashMap<>();
pFeatures.forEach((k,v) -> features.put(k.toString(), Boolean.valueOf(v.toString())));
config.put(XMLReaderPool.XmlParser.XML_PARSER_FEATURES_PROPERTY, features);
}
}
}
}

private void configureHtmlToXmlParser(final Element parser) {
final NodeList nlHtmlToXml = parser.getElementsByTagName(HtmlToXmlParser.HTML_TO_XML_PARSER_ELEMENT);
if(nlHtmlToXml.getLength() > 0) {
final Element htmlToXml = (Element)nlHtmlToXml.item(0);
Expand Down
37 changes: 34 additions & 3 deletions src/org/exist/util/XMLReaderPool.java
Expand Up @@ -27,12 +27,16 @@
import org.exist.Namespaces;
import org.exist.storage.BrokerPool;
import org.exist.storage.BrokerPoolService;
import org.exist.storage.BrokerPoolServiceException;
import org.exist.validation.GrammarPool;
import org.xml.sax.SAXNotRecognizedException;
import org.xml.sax.SAXNotSupportedException;
import org.xml.sax.XMLReader;
import org.xml.sax.ext.DefaultHandler2;

import javax.xml.parsers.ParserConfigurationException;
import java.util.Map;

/**
* Maintains a pool of XMLReader objects. The pool is available through
* {@link BrokerPool#getParserPool()}.
Expand All @@ -45,25 +49,46 @@ public class XMLReaderPool extends StackObjectPool<XMLReader> implements BrokerP

private final static DefaultHandler2 DUMMY_HANDLER = new DefaultHandler2();

private Configuration configuration = null;

/**
*
*
* @param factory
* @param maxIdle
* @param initIdleCapacity
*/
public XMLReaderPool(PoolableObjectFactory<XMLReader> factory, int maxIdle, int initIdleCapacity) {
public XMLReaderPool(final PoolableObjectFactory<XMLReader> factory, final int maxIdle, final int initIdleCapacity) {
super(factory, maxIdle, initIdleCapacity);
}

@Override
public void configure(final Configuration configuration) throws BrokerPoolServiceException {
this.configuration = configuration;
}

public synchronized XMLReader borrowXMLReader() {
try {
return super.borrowObject();
final XMLReader reader = super.borrowObject();
setParserConfigFeatures(reader);
return reader;
} catch (final Exception e) {
throw new IllegalStateException("error while returning XMLReader: " + e.getMessage(), e );
}
}

/**
* Sets any features for the parser which were defined in conf.xml
*/
private void setParserConfigFeatures(final XMLReader xmlReader) throws ParserConfigurationException, SAXNotRecognizedException, SAXNotSupportedException {
final Map<String, Boolean> parserFeatures = (Map<String, Boolean>)configuration.getProperty(XmlParser.XML_PARSER_FEATURES_PROPERTY);
if(parserFeatures != null) {
for(final Map.Entry<String, Boolean> feature : parserFeatures.entrySet()) {
xmlReader.setFeature(feature.getKey(), feature.getValue());
}
}
}

@Override
public synchronized XMLReader borrowObject() throws Exception {
return borrowXMLReader();
Expand Down Expand Up @@ -115,6 +140,12 @@ private Object getReaderProperty(XMLReader xmlReader, String propertyName){
return object;
}



// just used for config properties
public interface XmlParser {
String XML_PARSER_ELEMENT = "xml";
String XML_PARSER_FEATURES_ELEMENT = "features";
String XML_PARSER_FEATURES_PROPERTY = "parser.xml-parser.features";
}

}