From cb89b19d7274a63d269de0041aa668730fc06894 Mon Sep 17 00:00:00 2001 From: Thomas Nymand Date: Thu, 13 Aug 2026 16:50:31 +0200 Subject: [PATCH] REF-32: Replace the JAXB runtime and reject document type declarations in OIOBPP OIOBPPUtil.parse returned null for every privilege list, and had done so on any JDK 16 or later: com.sun.xml.bind 2.3.0 generates accessors by reflecting into ClassLoader.defineClass, which those JDKs refuse, and the resulting exception is swallowed by the catch in parse. Privileges from the assertion were silently dropped, and OIOBPPUtilTest was the visible symptom. Replace com.sun.xml.bind jaxb-core and jaxb-impl 2.3.0 with the reference implementation under its current coordinates, org.glassfish.jaxb:jaxb-runtime 2.3.9, and move jaxb-api to 2.3.1. This stays on the javax.xml.bind API and the artifact is still Java 8 bytecode. The parser also accepted a document type declaration, so a short privilege list could expand into a very large document while being parsed. External entities were already turned off, which does not cover entities declared inside the document. disallow-doctype-decl and FEATURE_SECURE_PROCESSING are now set. The privilege list is taken from an assertion whose signature has already been validated, so both of these are behind an authenticated boundary. Tested with a privilege list carrying a document type declaration, which is now refused. The whole suite passes for the first time, 113 tests without failures. --- oiosaml/pom.xml | 17 ++++++-------- .../dk/gov/oio/saml/oiobpp/OIOBPPUtil.java | 6 +++++ .../gov/oio/saml/oiobpp/OIOBPPUtilTest.java | 22 +++++++++++++++++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/oiosaml/pom.xml b/oiosaml/pom.xml index 9f58a0c..79cee13 100644 --- a/oiosaml/pom.xml +++ b/oiosaml/pom.xml @@ -163,19 +163,16 @@ javax.xml.bind jaxb-api - 2.3.0 + 2.3.1 + - com.sun.xml.bind - jaxb-core - 2.3.0 - - - - com.sun.xml.bind - jaxb-impl - 2.3.0 + org.glassfish.jaxb + jaxb-runtime + 2.3.9 diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/oiobpp/OIOBPPUtil.java b/oiosaml/src/main/java/dk/gov/oio/saml/oiobpp/OIOBPPUtil.java index 68cbf9e..f9f76f2 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/oiobpp/OIOBPPUtil.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/oiobpp/OIOBPPUtil.java @@ -4,6 +4,7 @@ import java.nio.charset.Charset; import java.util.Base64; +import javax.xml.XMLConstants; import javax.xml.bind.JAXBContext; import javax.xml.bind.JAXBElement; import javax.xml.bind.JAXBException; @@ -59,6 +60,11 @@ private static Source getSecureSource(String object) throws JAXBException { private static SAXParserFactory getSecureSAXParserFactory() throws SAXNotRecognizedException, SAXNotSupportedException, ParserConfigurationException { SAXParserFactory spf = SAXParserFactory.newInstance(); spf.setNamespaceAware(true); + + // Rejecting the document type declaration outright also rules out internal entities, which external + // entity handling alone does not cover + spf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + spf.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); spf.setFeature("http://xml.org/sax/features/external-general-entities", false); spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); spf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/oiobpp/OIOBPPUtilTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/oiobpp/OIOBPPUtilTest.java index 9b0ce42..9a85369 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/oiobpp/OIOBPPUtilTest.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/oiobpp/OIOBPPUtilTest.java @@ -29,6 +29,14 @@ public void testInvalidString() { Assertions.assertEquals(null, result); } + @DisplayName("Test OIOBPP string with a document type declaration") + @Test + public void testStringWithDoctype() { + PrivilegeList result = OIOBPPUtil.parse(doctypeString); + + Assertions.assertEquals(null, result); + } + private static final String validString = "\n" + "\n" + " \n" + @@ -42,6 +50,20 @@ public void testInvalidString() { " \n" + ""; + // Entities declared in the document type declaration expand into each other, so a short document turns + // into a very large one while it is parsed + private static final String doctypeString = "\n" + + "\n" + + " \n" + + " \n" + + "]>\n" + + "\n" + + " \n" + + " &c;\n" + + " \n" + + ""; + private static final String invalidString = "\n" + "\n" + " \n" +