This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/domsource in repository https://gitbox.apache.org/repos/asf/ws-xmlschema.git
commit ac4381bb0c12f64111ec002f9240ba1a5e04ecb3 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Fri Aug 28 09:18:53 2026 +0100 Fix DOMSource correctness bug --- THREAT-MODEL.md | 23 +++--- .../ws/commons/schema/XmlSchemaCollection.java | 15 +++- .../src/test/java/tests/ReadDomSourceTest.java | 93 ++++++++++++++++++++++ 3 files changed, 119 insertions(+), 12 deletions(-) diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index 4f242b88..39177fcb 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -149,7 +149,7 @@ A finding is in-model only if it reaches a row marked **yes**. | # | Transition | Authentication | Authorization | | --- | --- | --- | --- | -| B1 | Caller → `XmlSchemaCollection.read(InputSource | InputStream | Reader | URL | Document | Element)` | none — caller is trusted | none | +| B1 | Caller → `XmlSchemaCollection.read(InputSource | Reader | Source | Document | Element)` | none — caller is trusted | none | | B2 | `XmlSchemaCollection.read(InputSource, ...)` → hardened JDK `DocumentBuilder` | none | DOCTYPE rejected by default; external DTD/entity resolution disabled | | B3 | Schema parser → `URIResolver.resolveEntity(namespace, schemaLocation, baseUri)` | none | bundled `DefaultURIResolver` does **no host filtering**: it constructs `new URL(new URL(baseUri), schemaLocation)` and hands back an `InputSource` pointing at it | | B4 | Resolved `InputSource` → `XmlSchemaCollection.read(InputSource, ...)` (recursive) | none | none | @@ -159,7 +159,8 @@ A finding is in-model only if it reaches a row marked **yes**. ### Reachability preconditions per family - **`xmlschema-core` parser** (`XmlSchemaCollection.read(InputSource)`, - `.read(InputStream)`, `.read(Reader)`): in-model when the bytes are + `.read(Reader)`, `.read(Source)` for non-`DOMSource` sources): in-model + when the bytes are attacker-controllable. XMLSchema sets `FEATURE_SECURE_PROCESSING=true` on its internal `DocumentBuilderFactory`, rejects DOCTYPE declarations by default, disables external general entities, external parameter @@ -265,7 +266,7 @@ points*: | `org.apache.ws.commons.schema.maxSchemaResolutions` system property | `1000` *(documented: `README.txt`)* | operator-tunable per-process limit | maximum schema documents resolved during one top-level read | | `org.apache.ws.commons.schema.maxNestingDepth` system property | `512` *(documented: `README.txt`)* | operator-tunable per-process limit | maximum structural nesting depth while building the schema model, including nested include/import/redefine document resolutions | | `org.apache.ws.commons.schema.allowDTD` system property | `false` *(documented: `README.txt`)* | compatibility toggle for operator-controlled deployments | allows DOCTYPE declarations in schema documents; external DTD and external entity resolution remain disabled | -| `DocumentBuilderFactory` provider | JDK default (typically Xerces fork) *(inferred — §14 Q6)* | depends on the JDK | shape of XML parsing for `read(InputSource)` / `read(InputStream)` paths | +| `DocumentBuilderFactory` provider | JDK default (typically Xerces fork) *(inferred — §14 Q6)* | depends on the JDK | shape of XML parsing for `read(InputSource)` / stream-shaped `read(Source)` paths | ### The insecure-default case @@ -279,7 +280,8 @@ required to install a restricting resolver per §10). XMLSchema's internal schema parser rejects DOCTYPE declarations by default and disables external DTD and external entity resolution. This -applies both to top-level `read(InputSource | InputStream | Reader)` +applies both to top-level `read(InputSource | Reader | Source)` non-DOMSource +paths parses and to recursive import/include/redefine reparses. Operators may set `org.apache.ws.commons.schema.allowDTD=true` to accept DOCTYPE-bearing schemas for compatibility; external DTD and external @@ -292,9 +294,8 @@ entity resolution remain disabled in that mode. | Entry point | Parameter | Attacker-controllable? | Caller must enforce | | --- | --- | --- | --- | | `XmlSchemaCollection.read(InputSource is)` | `is` bytes | **yes** | XMLSchema rejects DOCTYPE by default and disables external DTD/entity resolution; caller may need to install a restricting `URIResolver` if the source contains untrusted `xs:include`/`xs:import` | -| `XmlSchemaCollection.read(InputStream in)` | `in` bytes | **yes** | same as above | | `XmlSchemaCollection.read(Reader r)` | `r` characters | **yes** | same as above | -| `XmlSchemaCollection.read(URL url)` | `url` | caller-supplied | caller controls; XMLSchema fetches via JDK URL handlers | +| `XmlSchemaCollection.read(Source src)` | `src` | **yes** | `SAXSource`/`StreamSource`/other route through the internal hardened factory (same as `read(InputSource)`); a `DOMSource` routes to the pre-parsed `read(Document)`/`read(Element)` path, so the upstream parser's XXE/DTD posture applies | | `XmlSchemaCollection.read(Document doc)` | `doc` | **yes if doc was parsed from untrusted bytes** | caller's `DocumentBuilderFactory` is responsible for XXE / DTD posture; XMLSchema does not re-parse | | `XmlSchemaCollection.read(Element el)` | `el` | same as `read(Document)` | same as above | | `XmlSchemaCollection.setSchemaResolver(URIResolver)` | resolver | caller-supplied | replacing the default is the documented path for production hardening *(inferred — §14 Q12)* | @@ -368,8 +369,8 @@ entity resolution remain disabled in that mode. ### P2 — Internal parser DTD and external-entity hardening -- **Condition**: XMLSchema's `read(InputSource | InputStream | Reader)` - paths take the internal `DocumentBuilderFactory`. +- **Condition**: XMLSchema's `read(InputSource | Reader | Source)` + non-DOMSource paths take the internal `DocumentBuilderFactory`. - **Property**: DOCTYPE declarations are rejected by default. External general entities, external parameter entities, external DTD loading, and JAXP external-DTD access are disabled; a no-op `EntityResolver` is @@ -625,8 +626,10 @@ model, the section that licenses the call. top-level `w3c-testcases/`)*. → `OUT-OF-MODEL: unsupported-component`. - **"`ExtensionRegistry` is `Class.forName`-loaded — RCE."** Trusted system property per §3 item 7. → `OUT-OF-MODEL: trusted-input`. -- **"`XmlSchemaCollection.read(URL)` follows `file://` to read - arbitrary local files."** Caller passed the URL; trusted entry point. +- **"`XmlSchemaCollection.read(InputSource)` with a `file://` system ID + follows it to read arbitrary local files."** (No `read(URL)` or + `read(InputStream)` overload exists; earlier revisions of this table + listed both in error.) Caller passed the URL; trusted entry point. → `OUT-OF-MODEL: trusted-input`. ## §12 Conditions that would change this model diff --git a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java index d20c08bb..8eb23785 100644 --- a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java +++ b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/XmlSchemaCollection.java @@ -633,6 +633,12 @@ public final class XmlSchemaCollection { /** * Read an XML schema into the collection from a TRaX source. Schemas in a collection must be unique in * the concatenation of system ID and targetNamespace. In this API, the systemID is taken from the Source. + * <p> + * Note: a {@link DOMSource} is read through the pre-parsed {@link #read(Document)} / + * {@link #read(Element)} path, so the XXE/DTD posture of that parse is whatever the upstream + * parser that produced the DOM was configured with — this collection's internally hardened + * parser is not involved. All other source types are parsed with the internal hardened parser. + * </p> * * @param source the XSD document. * @return the XML schema object. @@ -642,10 +648,15 @@ public final class XmlSchemaCollection { return read(((SAXSource)source).getInputSource()); } else if (source instanceof DOMSource) { Node node = ((DOMSource)source).getNode(); + String systemId = source.getSystemId(); if (node instanceof Document) { - node = ((Document)node).getDocumentElement(); + return read((Document)node, systemId); + } else if (node instanceof Element) { + return read((Element)node, systemId); } - return read((Document)node); + throw new XmlSchemaException("A DOMSource must wrap a Document or an Element node, but " + + (node == null ? "no node was provided." + : "a " + node.getClass().getName() + " was provided.")); } else if (source instanceof StreamSource) { StreamSource ss = (StreamSource)source; InputSource isource = new InputSource(ss.getSystemId()); diff --git a/xmlschema-core/src/test/java/tests/ReadDomSourceTest.java b/xmlschema-core/src/test/java/tests/ReadDomSourceTest.java new file mode 100644 index 00000000..b14cb60a --- /dev/null +++ b/xmlschema-core/src/test/java/tests/ReadDomSourceTest.java @@ -0,0 +1,93 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package tests; + +import java.io.StringReader; + +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.transform.dom.DOMSource; + +import org.apache.ws.commons.schema.XmlSchema; +import org.apache.ws.commons.schema.XmlSchemaCollection; +import org.apache.ws.commons.schema.XmlSchemaException; + +import org.junit.Assert; +import org.junit.Test; + +import org.w3c.dom.Document; + +import org.xml.sax.InputSource; + +/** + * read(Source) with a DOMSource must route to the pre-parsed read paths + * (previously it threw ClassCastException for every DOMSource), and must + * reject DOMSources wrapping anything but a Document or Element. + */ +public class ReadDomSourceTest extends Assert { + + private static final String SCHEMA = + "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\""; + + " targetNamespace=\"urn:domsource\">" + + "<xs:element name=\"e\" type=\"xs:string\"/>" + + "</xs:schema>"; + + private Document parse() throws Exception { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + return factory.newDocumentBuilder().parse(new InputSource(new StringReader(SCHEMA))); + } + + @Test + public void testDomSourceWrappingDocument() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + XmlSchema schema = collection.read(new DOMSource(parse())); + assertNotNull(schema); + assertEquals("urn:domsource", schema.getTargetNamespace()); + } + + @Test + public void testDomSourceWrappingElement() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + XmlSchema schema = collection.read(new DOMSource(parse().getDocumentElement())); + assertNotNull(schema); + assertEquals("urn:domsource", schema.getTargetNamespace()); + } + + @Test + public void testDomSourceSystemIdIsPreserved() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + DOMSource source = new DOMSource(parse()); + source.setSystemId("urn:domsource:system-id"); + XmlSchema schema = collection.read(source); + assertEquals("urn:domsource:system-id", schema.getSourceURI()); + } + + @Test + public void testDomSourceWrappingOtherNodeIsRejected() throws Exception { + XmlSchemaCollection collection = new XmlSchemaCollection(); + Document doc = parse(); + try { + collection.read(new DOMSource(doc.createTextNode("not a schema"))); + fail("A DOMSource wrapping a text node should be rejected."); + } catch (XmlSchemaException expected) { + // documented failure surface (previously ClassCastException) + } + } +} \ No newline at end of file
