diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index a2617bcd752..f26c2c4c252 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -132,6 +132,7 @@ public static void addComment(XmlTokenSource doc, String comment) public static final SAXParserFactory SAX_PARSER_FACTORY_ALLOWING_DOCTYPE; public static final XMLInputFactory XML_INPUT_FACTORY; public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY; + public static final DocumentBuilderFactory DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE; static { @@ -145,16 +146,9 @@ public static void addComment(XmlTokenSource doc, String comment) SAX_PARSER_FACTORY = saxParserFactory(false); SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true); - //noinspection XMLInputFactory - DOCUMENT_BUILDER_FACTORY = DocumentBuilderFactory.newInstance(); - DOCUMENT_BUILDER_FACTORY.setNamespaceAware(true); - DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-general-entities", false); - DOCUMENT_BUILDER_FACTORY.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - DOCUMENT_BUILDER_FACTORY.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - DOCUMENT_BUILDER_FACTORY.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - DOCUMENT_BUILDER_FACTORY.setXIncludeAware(false); - DOCUMENT_BUILDER_FACTORY.setExpandEntityReferences(false); + DOCUMENT_BUILDER_FACTORY = documentBuilderFactory(false); + // Use the ALLOWING_DOCTYPE variant when parsing XML that contains a declaration (e.g. NCBI's eSummary responses) + DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true); } catch (ParserConfigurationException | SAXException e) { @@ -181,4 +175,26 @@ private static SAXParserFactory saxParserFactory(boolean allowDocType) throws SA result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); return result; } + + private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocType) throws ParserConfigurationException + { + //noinspection XMLInputFactory + DocumentBuilderFactory result = DocumentBuilderFactory.newInstance(); + result.setNamespaceAware(true); + + // Disable features that could lead to XXE or other vulnerabilities. + // When allowDocType is true the DOCTYPE declaration is permitted. External entity + // resolution remains disabled, so XXE protection is still in effect. + if (!allowDocType) + { + result.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + } + result.setFeature("http://xml.org/sax/features/external-general-entities", false); + result.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + result.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + result.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + result.setXIncludeAware(false); + result.setExpandEntityReferences(false); + return result; + } }