WnLmfReader: adopt LexicalKnowledgeBase and safe-skip WN-LMF DOCTYPE declarations

Completes the LexicalKnowledgeBase rename in this file, and switches
DOCTYPE handling from outright rejection to the OWASP-documented
alternative for formats that require DOCTYPE support: the DOCTYPE
event is skipped rather than resolved, while SUPPORT_DTD stays off
(so no custom entity, internal or external, is ever declared),
IS_SUPPORTING_EXTERNAL_ENTITIES and ACCESS_EXTERNAL_DTD stay off, and
the XMLResolver still throws on any resolution attempt. This closes
the same attack surface as outright rejection while letting the
reader parse Open English WordNet releases unmodified, since real
OEWN files ship a DOCTYPE line referencing the schema DTD.

Test changes: testRejectsDoctype becomes testSkipsDoctypeDeclaration
and now asserts a DOCTYPE-bearing document parses correctly instead
of throwing. Added testInternalSubsetEntityIsNeverExpanded, which
proves a classic XXE payload (a DOCTYPE-declared internal-subset
entity pointing at a local file) still fails loud rather than
expanding into the parsed output.

Author: krickert

opennlp-extensions/opennlp-wordnet/src/main/java/opennlp/wordnet/WnLmfReader.java

@@ -28,23 +28,24 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import javax.xml.XMLConstants;
 import javax.xml.stream.Location;
 import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamConstants;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.XMLStreamReader;
 
+import opennlp.tools.wordnet.LexicalKnowledgeBase;
 import opennlp.tools.wordnet.Synset;
-import opennlp.tools.wordnet.WordNetLexicon;
 import opennlp.tools.wordnet.WordNetPos;
 import opennlp.tools.wordnet.WordNetRelation;
 
 /**
  * Reads a WN-LMF XML document (the Global WordNet Association interchange format, used by Open
- * English WordNet and many other language wordnets) into a {@link WordNetLexicon}.
+ * English WordNet and many other language wordnets) into a {@link LexicalKnowledgeBase}.
  *
  * <p>The reader is clean-room, built from the published format documentation, and uses only the
- * JDK's StAX parser. It reads the subset of the format the {@link WordNetLexicon} contract
+ * JDK's StAX parser. It reads the subset of the format the {@link LexicalKnowledgeBase} contract
  * serves: lexical entries (lemma, part of speech, senses), synsets (definition and typed
  * relations), and sense relations, which are lifted to the synset level as documented on
  * {@link WordNetRelation}. Elements outside that subset ({@code Pronunciation}, {@code Form},
@@ -53,12 +54,14 @@
  * metadata, are also skipped: the contract has no untyped relation slot. Every other unknown
  * relation type fails loud.</p>
  *
- * <p><b>Security.</b> The parser is hardened against XXE: DTD processing and external entity
- * resolution are disabled, nothing is ever fetched from the network, and a document containing
- * a DOCTYPE declaration is rejected outright. Note that Open English WordNet releases ship with
- * a DOCTYPE line referencing the schema DTD; to read such a file with this v1 reader, delete
- * that one line first. Rejecting DTDs wholesale rather than trusting parser-specific partial
- * DTD support is a deliberate v1 posture.</p>
+ * <p><b>Security.</b> The parser is hardened against XXE per the OWASP-documented posture for
+ * formats that carry a DOCTYPE: a DOCTYPE declaration is tokenized and skipped, but nothing it
+ * names is ever resolved. External entities and the external DTD subset are both disabled
+ * ({@code IS_SUPPORTING_EXTERNAL_ENTITIES} and {@code ACCESS_EXTERNAL_DTD} are off), and the
+ * {@link javax.xml.stream.XMLResolver} throws on any resolution attempt regardless, so no
+ * network or filesystem access can be triggered by a DOCTYPE, internal or external. Open English
+ * WordNet releases ship with a DOCTYPE line referencing the schema DTD; this reader parses such
+ * a file unmodified.</p>
  *
  * <p><b>Errors.</b> Malformed structure fails loud with an {@link IllegalArgumentException}
  * naming the resource and, where the parser provides one, the line: missing required
@@ -92,7 +95,7 @@ private WnLmfReader() {
    *     document is malformed; the message names the file and, where available, the line.
    * @throws UncheckedIOException Thrown if reading the file fails.
    */
-  public static WordNetLexicon read(Path file) {
+  public static LexicalKnowledgeBase read(Path file) {
     if (file == null) {
       throw new IllegalArgumentException("File must not be null");
     }
@@ -115,7 +118,7 @@ public static WordNetLexicon read(Path file) {
    * @throws IllegalArgumentException Thrown if an argument is {@code null} or the document is
    *     malformed; the message names the resource and, where available, the line.
    */
-  public static WordNetLexicon read(InputStream in, String resourceName) {
+  public static LexicalKnowledgeBase read(InputStream in, String resourceName) {
     if (in == null) {
       throw new IllegalArgumentException("In must not be null");
     }
@@ -138,9 +141,13 @@ public static WordNetLexicon read(InputStream in, String resourceName) {
 
   private static XMLInputFactory hardenedFactory() {
     final XMLInputFactory factory = XMLInputFactory.newFactory();
-    // XXE hardening: no DTD processing, no external entities, nothing fetched from anywhere.
+    // XXE hardening, the OWASP-documented posture for DOCTYPE-bearing formats: the internal
+    // subset is not processed (so no custom entity gets declared at all), and both the
+    // external DTD subset and external entities are denied, so a DOCTYPE is tokenized and
+    // skipped but nothing it names is ever fetched.
     factory.setProperty(XMLInputFactory.SUPPORT_DTD, Boolean.FALSE);
     factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE);
+    factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
     factory.setProperty(XMLInputFactory.IS_COALESCING, Boolean.TRUE);
     factory.setXMLResolver((publicId, systemId, baseUri, namespace) -> {
       throw new XMLStreamException("External entity resolution is disabled, refusing " + systemId);
@@ -178,10 +185,10 @@ private static final class Parser {
     void parse(XMLStreamReader reader) throws XMLStreamException {
       while (reader.hasNext()) {
         final int event = reader.next();
-        if (event == XMLStreamConstants.DTD) {
-          throw malformed(reader.getLocation(),
-              "Document contains a DOCTYPE declaration; the hardened reader rejects DTDs", null);
-        }
+        // A DTD event (the DOCTYPE declaration) is intentionally not handled here: with
+        // external entities and the external subset denied in the factory, it carries nothing
+        // that can affect parsing, so it is skipped exactly like a comment or an ignored
+        // element.
         if (event == XMLStreamConstants.START_ELEMENT) {
           startElement(reader);
         } else if (event == XMLStreamConstants.END_ELEMENT) {
@@ -277,7 +284,7 @@ private void endElement(String name) {
       }
     }
 
-    WordNetLexicon build() {
+    LexicalKnowledgeBase build() {
       // Every sense must point to a declared synset, with a consistent part of speech.
       for (final Map.Entry<String, String> sense : synsetBySenseId.entrySet()) {
         final RawSynset target = rawSynsets.get(sense.getValue());

opennlp-extensions/opennlp-wordnet/src/test/java/opennlp/wordnet/WnLmfReaderTest.java

@@ -27,8 +27,8 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
 
+import opennlp.tools.wordnet.LexicalKnowledgeBase;
 import opennlp.tools.wordnet.Synset;
-import opennlp.tools.wordnet.WordNetLexicon;
 import opennlp.tools.wordnet.WordNetPos;
 import opennlp.tools.wordnet.WordNetRelation;
 
@@ -40,7 +40,7 @@
 
 public class WnLmfReaderTest {
 
-  static WordNetLexicon fixture() {
+  static LexicalKnowledgeBase fixture() {
     try (InputStream in = WnLmfReaderTest.class.getResourceAsStream("mini-wn-lmf.xml")) {
       assertNotNull(in, "Fixture mini-wn-lmf.xml must be on the test classpath");
       return WnLmfReader.read(in, "mini-wn-lmf.xml");
@@ -49,7 +49,7 @@ static WordNetLexicon fixture() {
     }
   }
 
-  private static WordNetLexicon parse(String document) {
+  private static LexicalKnowledgeBase parse(String document) {
     return WnLmfReader.read(
         new ByteArrayInputStream(document.getBytes(StandardCharsets.UTF_8)), "inline.xml");
   }
@@ -74,7 +74,7 @@ void testLookupReturnsSynsetWithAllComponents() {
 
   @Test
   void testLookupFoldsCaseAndUnderscore() {
-    final WordNetLexicon lexicon = fixture();
+    final LexicalKnowledgeBase lexicon = fixture();
     assertEquals("mini-n1", lexicon.lookup("Domestic_Dog", WordNetPos.NOUN).get(0).id());
     assertEquals("mini-n1", lexicon.lookup("DOG", WordNetPos.NOUN).get(0).id());
   }
@@ -88,7 +88,7 @@ void testLookupKeepsSenseOrder() {
 
   @Test
   void testLookupIsPosScoped() {
-    final WordNetLexicon lexicon = fixture();
+    final LexicalKnowledgeBase lexicon = fixture();
     assertEquals(1, lexicon.lookup("run", WordNetPos.VERB).size());
     assertTrue(lexicon.lookup("dog", WordNetPos.VERB).isEmpty());
     assertFalse(lexicon.contains("walk", WordNetPos.NOUN));
@@ -97,7 +97,7 @@ void testLookupIsPosScoped() {
 
   @Test
   void testRelationNavigation() {
-    final WordNetLexicon lexicon = fixture();
+    final LexicalKnowledgeBase lexicon = fixture();
     assertEquals(List.of("mini-n1"), lexicon.related("mini-n2", WordNetRelation.HYPONYM));
     assertEquals(List.of("mini-v1", "mini-v2"),
         lexicon.related("mini-v4", WordNetRelation.HYPONYM));
@@ -106,7 +106,7 @@ void testRelationNavigation() {
 
   @Test
   void testSenseRelationsAreLiftedToSynsetLevel() {
-    final WordNetLexicon lexicon = fixture();
+    final LexicalKnowledgeBase lexicon = fixture();
     assertEquals(List.of("mini-a2"), lexicon.related("mini-a1", WordNetRelation.ANTONYM));
     assertEquals(List.of("mini-a1"), lexicon.related("mini-a2", WordNetRelation.ANTONYM));
     assertEquals(List.of("mini-v1"),
@@ -126,7 +126,7 @@ void testSatelliteNormalizesToAdjective() {
 
   @Test
   void testUnknownLemmaOrSynsetIsEmpty() {
-    final WordNetLexicon lexicon = fixture();
+    final LexicalKnowledgeBase lexicon = fixture();
     assertTrue(lexicon.lookup("zebra", WordNetPos.NOUN).isEmpty());
     assertTrue(lexicon.synset("mini-n99").isEmpty());
   }
@@ -138,7 +138,7 @@ void testReadPath(@TempDir Path tempDir) throws IOException {
         "<LexicalEntry id=\"t-cat-n\"><Lemma writtenForm=\"cat\" partOfSpeech=\"n\"/>"
             + "<Sense id=\"t-cat-n-1\" synset=\"t-1\"/></LexicalEntry>"
             + "<Synset id=\"t-1\" partOfSpeech=\"n\"><Definition>a feline</Definition></Synset>"));
-    final WordNetLexicon lexicon = WnLmfReader.read(file);
+    final LexicalKnowledgeBase lexicon = WnLmfReader.read(file);
     assertEquals("a feline", lexicon.lookup("cat", WordNetPos.NOUN).get(0).gloss());
   }
 
@@ -157,14 +157,39 @@ void testReadStreamRejectsNulls() {
   }
 
   @Test
-  void testRejectsDoctype() {
+  void testSkipsDoctypeDeclaration() {
+    // Real Open English WordNet releases ship exactly this shape: a DOCTYPE naming the schema
+    // DTD by an unreachable SYSTEM identifier (example.invalid is the RFC 2606 reserved domain
+    // that must never resolve). The reader must parse past it without attempting to fetch it.
     final String document = "<?xml version=\"1.0\"?>\n"
         + "<!DOCTYPE LexicalResource SYSTEM \"http://example.invalid/WN-LMF-1.3.dtd\">\n"
         + "<LexicalResource><Lexicon id=\"t\" label=\"t\" language=\"en\" version=\"1\">"
+        + "<LexicalEntry id=\"t-cat-n\"><Lemma writtenForm=\"cat\" partOfSpeech=\"n\"/>"
+        + "<Sense id=\"t-cat-n-1\" synset=\"t-1\"/></LexicalEntry>"
+        + "<Synset id=\"t-1\" partOfSpeech=\"n\"><Definition>a feline</Definition></Synset>"
+        + "</Lexicon></LexicalResource>";
+    final LexicalKnowledgeBase lexicon = parse(document);
+    assertEquals("a feline", lexicon.lookup("cat", WordNetPos.NOUN).get(0).gloss());
+  }
+
+  @Test
+  void testInternalSubsetEntityIsNeverExpanded(@TempDir Path tempDir) throws IOException {
+    // A DOCTYPE-declared internal-subset entity is the classic XXE payload: if the parser ever
+    // honored it, the entity reference below would be replaced by the target file's content.
+    // With SUPPORT_DTD disabled the declaration itself is never registered, so the reference is
+    // undefined and parsing must fail loud rather than silently expand it.
+    final Path secret = tempDir.resolve("secret.txt");
+    Files.writeString(secret, "xxe-marker-should-never-appear");
+    final String document = "<?xml version=\"1.0\"?>\n"
+        + "<!DOCTYPE LexicalResource [<!ENTITY xxe SYSTEM \"" + secret.toUri() + "\">]>\n"
+        + "<LexicalResource><Lexicon id=\"t\" label=\"t\" language=\"en\" version=\"1\">"
+        + "<LexicalEntry id=\"t-cat-n\"><Lemma writtenForm=\"&xxe;\" partOfSpeech=\"n\"/>"
+        + "<Sense id=\"t-cat-n-1\" synset=\"t-1\"/></LexicalEntry>"
+        + "<Synset id=\"t-1\" partOfSpeech=\"n\"><Definition>a feline</Definition></Synset>"
         + "</Lexicon></LexicalResource>";
     final IllegalArgumentException e =
         assertThrows(IllegalArgumentException.class, () -> parse(document));
-    assertTrue(e.getMessage().contains("inline.xml"));
+    assertFalse(e.getMessage().contains("xxe-marker-should-never-appear"));
   }
 
   @Test
@@ -212,7 +237,7 @@ void testRejectsUnknownRelationType() {
 
   @Test
   void testSkipsOtherRelationType() {
-    final WordNetLexicon lexicon = parse(
+    final LexicalKnowledgeBase lexicon = parse(
         wrap("<LexicalEntry id=\"t-cat-n\"><Lemma writtenForm=\"cat\" partOfSpeech=\"n\"/>"
             + "<Sense id=\"t-cat-n-1\" synset=\"t-1\">"
             + "<SenseRelation relType=\"other\" target=\"t-cat-n-1\"/></Sense></LexicalEntry>"