From b1f5bceac5ab9d956b3966c5a049b7dbd9512568 Mon Sep 17 00:00:00 2001 From: Vagisha Sharma Date: Tue, 7 Apr 2026 18:35:16 -0700 Subject: [PATCH 1/4] - Added DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE to XmlBeansUtis. Mirrors the existing SAX_PARSER_FACTORY_ALLOWING_DOCTYPE: permits the DOCTYPE declaration but keeps every other XXE mitigation in place - Extracted a private documentBuilderFactory(boolean allowDocType) helper so both DOM factories share one configuration site, mirroring the existing saxParserFactory(boolean) helper --- api/src/org/labkey/api/util/XmlBeansUtil.java | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index a2617bcd752..6960b73e8c1 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,8 @@ 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); + DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true); } catch (ParserConfigurationException | SAXException e) { @@ -181,4 +174,30 @@ 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, but external entity + // and external DTD resolution remain disabled, so XXE protection stays in effect — + // only the strict disallow-doctype-decl flag is relaxed. Use the ALLOWING_DOCTYPE + // variant when parsing XML from a source that legitimately emits a + // declaration (e.g. NCBI's eSummary responses, which reference an external DTD that + // we don't actually fetch). + 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; + } } From 13a755e568e96139b24797e74779e761763589f9 Mon Sep 17 00:00:00 2001 From: Vagisha Sharma Date: Tue, 7 Apr 2026 19:38:11 -0700 Subject: [PATCH 2/4] Fixed comments --- api/src/org/labkey/api/util/XmlBeansUtil.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 6960b73e8c1..f26c2c4c252 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -147,6 +147,7 @@ public static void addComment(XmlTokenSource doc, String comment) SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true); 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) @@ -182,12 +183,8 @@ private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocTyp result.setNamespaceAware(true); // Disable features that could lead to XXE or other vulnerabilities. - // When allowDocType is true the DOCTYPE declaration is permitted, but external entity - // and external DTD resolution remain disabled, so XXE protection stays in effect — - // only the strict disallow-doctype-decl flag is relaxed. Use the ALLOWING_DOCTYPE - // variant when parsing XML from a source that legitimately emits a - // declaration (e.g. NCBI's eSummary responses, which reference an external DTD that - // we don't actually fetch). + // 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); From 25614349b17140fff8eb0925916209eeec94ad3c Mon Sep 17 00:00:00 2001 From: Vagisha Sharma Date: Tue, 7 Apr 2026 18:35:16 -0700 Subject: [PATCH 3/4] - Added DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE to XmlBeansUtis. Mirrors the existing SAX_PARSER_FACTORY_ALLOWING_DOCTYPE: permits the DOCTYPE declaration but keeps every other XXE mitigation in place - Extracted a private documentBuilderFactory(boolean allowDocType) helper so both DOM factories share one configuration site, mirroring the existing saxParserFactory(boolean) helper --- api/src/org/labkey/api/util/XmlBeansUtil.java | 39 ++++++++++++++----- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index a2617bcd752..6960b73e8c1 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,8 @@ 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); + DOCUMENT_BUILDER_FACTORY_ALLOWING_DOCTYPE = documentBuilderFactory(true); } catch (ParserConfigurationException | SAXException e) { @@ -181,4 +174,30 @@ 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, but external entity + // and external DTD resolution remain disabled, so XXE protection stays in effect — + // only the strict disallow-doctype-decl flag is relaxed. Use the ALLOWING_DOCTYPE + // variant when parsing XML from a source that legitimately emits a + // declaration (e.g. NCBI's eSummary responses, which reference an external DTD that + // we don't actually fetch). + 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; + } } From 0ef50f4f12a31cad38f3abe756998ac1c4fbca60 Mon Sep 17 00:00:00 2001 From: Vagisha Sharma Date: Tue, 7 Apr 2026 19:38:11 -0700 Subject: [PATCH 4/4] Fixed comments --- api/src/org/labkey/api/util/XmlBeansUtil.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/api/src/org/labkey/api/util/XmlBeansUtil.java b/api/src/org/labkey/api/util/XmlBeansUtil.java index 6960b73e8c1..f26c2c4c252 100644 --- a/api/src/org/labkey/api/util/XmlBeansUtil.java +++ b/api/src/org/labkey/api/util/XmlBeansUtil.java @@ -147,6 +147,7 @@ public static void addComment(XmlTokenSource doc, String comment) SAX_PARSER_FACTORY_ALLOWING_DOCTYPE = saxParserFactory(true); 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) @@ -182,12 +183,8 @@ private static DocumentBuilderFactory documentBuilderFactory(boolean allowDocTyp result.setNamespaceAware(true); // Disable features that could lead to XXE or other vulnerabilities. - // When allowDocType is true the DOCTYPE declaration is permitted, but external entity - // and external DTD resolution remain disabled, so XXE protection stays in effect — - // only the strict disallow-doctype-decl flag is relaxed. Use the ALLOWING_DOCTYPE - // variant when parsing XML from a source that legitimately emits a - // declaration (e.g. NCBI's eSummary responses, which reference an external DTD that - // we don't actually fetch). + // 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);