Re-apply entity/DTD hardening after JDK deserialization of XmlFactory

release-notes/CREDITS

@@ -154,5 +154,8 @@ Christian Beikov (@beikov)
   (3.3.0)
 
 @Sahana2524
- * Fixed #879:  Use `Locale.ROOT` for case folding in `CaseInsensitiveNameSet`
+ * Fixed #878: Re-apply entity/DTD hardening after JDK deserialization of
+   `XmlFactory`
+  (3.3.0)
+ * Fixed #879: Use `Locale.ROOT` for case folding in `CaseInsensitiveNameSet`
   (3.3.0)

release-notes/VERSION

@@ -9,6 +9,8 @@ Version: 3.x (for earlier see VERSION-2.x)
 
 #873: Fix handling of `@JsonApplyView`
  (fix by @cowtowncoder, w/ Claude code)
+#878: Re-apply entity/DTD hardening after JDK deserialization of `XmlFactory`
+ (fix by @Sahana2524)
 #879: Use `Locale.ROOT` for case folding in `CaseInsensitiveNameSet`
  (fix by @Sahana2524)
 

src/main/java/tools/jackson/dataformat/xml/XmlFactory.java

@@ -240,7 +240,12 @@ protected Object readResolve() {
         final XMLInputFactory inf;
         final XMLOutputFactory outf;
         try {
-            inf = (XMLInputFactory) Class.forName(_jdkXmlInFactory).getDeclaredConstructor().newInstance();
+            // Only the factory class name survives serialization, so we get a bare
+            // default instance back: re-apply the entity/DTD hardening the builder
+            // would have set, otherwise a securely-built factory comes back with
+            // external entity + DTD processing re-enabled (see [dataformat-xml#190], [dataformat-xml#211]).
+            inf = XmlFactoryBuilder.secureXmlInputFactory(
+                    (XMLInputFactory) Class.forName(_jdkXmlInFactory).getDeclaredConstructor().newInstance());
             outf = (XMLOutputFactory) Class.forName(_jdkXmlOutFactory).getDeclaredConstructor().newInstance();
         } catch (Exception e) {
             throw new IllegalArgumentException(e);

src/main/java/tools/jackson/dataformat/xml/XmlFactoryBuilder.java

@@ -119,6 +119,19 @@ protected static XMLInputFactory defaultXmlInputFactory(ClassLoader cl) {
             // 24-Oct-2022, tatu: as per [dataformat-xml#550] need extra care
             xmlIn = XMLInputFactory.newFactory();
         }
+        return secureXmlInputFactory(xmlIn);
+    }
+
+    /**
+     * Applies the default entity/DTD hardening to a freshly created
+     * {@link XMLInputFactory}. Shared so that any code path building a factory
+     * internally (including JDK-deserialization reconstruction in
+     * {@code XmlFactory.readResolve()}) applies the same protections and the two
+     * can not drift apart.
+     *
+     * @since 3.3
+     */
+    protected static XMLInputFactory secureXmlInputFactory(XMLInputFactory xmlIn) {
         // as per [dataformat-xml#190], disable external entity expansion by default
         xmlIn.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE);
         // and ditto wrt [dataformat-xml#211], SUPPORT_DTD

src/test/java/tools/jackson/dataformat/xml/misc/DTDAfterSerializationTest.java

@@ -0,0 +1,44 @@
+package tools.jackson.dataformat.xml.misc;
+
+import java.io.*;
+import java.util.Map;
+
+import org.junit.jupiter.api.Test;
+
+import tools.jackson.core.exc.StreamReadException;
+import tools.jackson.dataformat.xml.XmlMapper;
+import tools.jackson.dataformat.xml.XmlTestUtil;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+// [dataformat-xml]: entity/DTD hardening must survive JDK serialization of the mapper
+public class DTDAfterSerializationTest extends XmlTestUtil
+{
+    // Internal entity that only expands when DTD processing is enabled
+    private static final String ENTITY_XML =
+            "<?xml version='1.0'?><!DOCTYPE foo [ <!ENTITY x \"HELLO\"> ]>\n"
+            + "<foo>&x;</foo>";
+
+    private XmlMapper jdkRoundtrip(XmlMapper mapper) throws Exception {
+        ByteArrayOutputStream bytes = new ByteArrayOutputStream();
+        try (ObjectOutputStream os = new ObjectOutputStream(bytes)) {
+            os.writeObject(mapper);
+        }
+        try (ObjectInputStream is = new ObjectInputStream(
+                new ByteArrayInputStream(bytes.toByteArray()))) {
+            return (XmlMapper) is.readObject();
+        }
+    }
+
+    @Test
+    public void testDTDStaysDisabledAfterRoundtrip() throws Exception
+    {
+        XmlMapper mapper = jdkRoundtrip(new XmlMapper());
+        // Must fail specifically because the parser refuses the DTD-declared
+        // entity (DTD support off), leaving `&x;` unexpanded -- not for some
+        // unrelated binding reason. Before the fix this expanded to "HELLO".
+        StreamReadException e = assertThrows(StreamReadException.class,
+                () -> mapper.readValue(ENTITY_XML, Map.class));
+        verifyException(e, "Undeclared general entity", "entity");
+    }
+}