This is an automated email from the ASF dual-hosted git repository. robertlazarski pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/axis-axis2-java-core.git
commit 3ff914a61448b50c620e8e41a2c48980f23158b4 Author: Robert Lazarski <[email protected]> AuthorDate: Fri Sep 4 16:12:01 2026 -1000 Make two threat-model claims true, and scope what they do not cover SECURITY.md said every parser factory the framework creates disables DTDs and external entities, and that the deployment resolvers block file:. Neither held. The SAAJ and JAX-WS conversions now use the hardened factory, and file: is refused by the resolvers -- reachable in one of the four, where an absolute file: import passed the guard. The document now names what stays uncovered: build-time codegen factories, the deliberate hbm.xml DTD exception, and the TransformerFactory instances. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- SECURITY.md | 35 ++++++++++++++++++--- .../axis2/jaxws/handler/LogicalMessageImpl.java | 7 +++-- .../deployment/resolver/AARBasedWSDLLocator.java | 9 +++++- .../resolver/AARFileBasedURIResolver.java | 9 +++++- .../deployment/resolver/WarBasedWSDLLocator.java | 9 +++++- .../resolver/WarFileBasedURIResolver.java | 9 +++++- .../kernel/src/org/apache/axis2/util/XMLUtils.java | 5 +-- .../apache/axis2/deployment/URIResolverTest.java | 36 ++++++++++++++++++++++ .../description/builder/JAXWSRIWSDLGenerator.java | 4 +-- .../src/org/apache/axis2/saaj/SOAPBodyImpl.java | 6 ++-- .../src/org/apache/axis2/saaj/util/SAAJUtil.java | 6 ++-- src/site/markdown/release-notes/2.0.2.md | 10 ++++++ 12 files changed, 125 insertions(+), 20 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 7d0f123750..41c3f93a79 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -216,9 +216,25 @@ migration from `commons-fileupload` 1.x to `commons-fileupload2` in ## Existing Security Hardening -1. **XML parsing:** All `DocumentBuilderFactory` and `SAXParserFactory` - instances created by the framework disable DTDs and external entities - (`XMLUtils.java`, `SecureWSDLLocator.java`, `DefaultEntityResolver.java`). +1. **XML parsing (scoped accurately in 2.0.2):** The parser factories on the + runtime paths disable DTDs and external entities: + `XMLUtils.newSecureDocumentBuilderFactory` and `XMLUtils.newDocument`, + `SecureWSDLLocator`, `HardenedWSDLLocator`, `DefaultEntityResolver`, and the + SAAJ and JAX-WS conversions that were missed until 2.0.2 (`XMLUtils.toDOM`, + `SAAJUtil.toDOM`, `SOAPBodyImpl`, `LogicalMessageImpl`, + `JAXWSRIWSDLGenerator`). + + Earlier revisions of this document said *all* factories created by the + framework. That was not true and is not a promise to make: + + - The code-generation tooling creates its own factories. Its dedicated + entry points are hardened (see item 13 and `XSD2Java`), but the + build-time emitters and schema generators are not audited here. + - `HbmXmlIntrospector` allows DTDs deliberately, because hbm.xml files + declare one. That is a permanent exception, not a gap. + - `TransformerFactory` instances do not set `ACCESS_EXTERNAL_DTD` or + `ACCESS_EXTERNAL_STYLESHEET`. Restricting them can break stylesheets + that legitimately import, so it has not been done blind. 2. **WSDL import security (extended in 2.0.2):** wsdl4j parses with its own unhardened parser and fetches the whole import chain itself, so every @@ -243,8 +259,17 @@ migration from `commons-fileupload` 1.x to `commons-fileupload2` in remote base URI, is outside what is screened. 3. **Schema import security:** URI resolvers for AAR and WAR deployments - block HTTP/HTTPS/FTP/JAR/file scheme resolution to prevent SSRF via - xmlschema-core's `DefaultURIResolver`. + refuse HTTP/HTTPS/FTP/JAR/file scheme resolution, so an import cannot + reach the network or the server's filesystem through xmlschema-core's + `DefaultURIResolver`; a location that is not absolute is looked up inside + the archive. + + `file:` was added to the guard in 2.0.2. Only `AARBasedWSDLLocator` had + been letting one through: it counts `file:` as absolute, so an absolute + `file:` import passed the scheme check and fell through to the parent + resolver. In the other three the predicate does not count `file:` as + absolute, so such a location was already treated as an archive resource + name and never opened a filesystem path. 4. **Deserialization of externalized contexts (2.0.2):** There is no class allowlist, and one cannot be shipped: context externalization exists to diff --git a/modules/jaxws/src/org/apache/axis2/jaxws/handler/LogicalMessageImpl.java b/modules/jaxws/src/org/apache/axis2/jaxws/handler/LogicalMessageImpl.java index fc831aa89e..023ce9102c 100644 --- a/modules/jaxws/src/org/apache/axis2/jaxws/handler/LogicalMessageImpl.java +++ b/modules/jaxws/src/org/apache/axis2/jaxws/handler/LogicalMessageImpl.java @@ -21,6 +21,7 @@ package org.apache.axis2.jaxws.handler; import org.apache.axiom.om.util.StAXUtils; import org.apache.axiom.soap.SOAPEnvelope; +import org.apache.axis2.util.XMLUtils; import org.apache.axis2.jaxws.ExceptionFactory; import org.apache.axis2.jaxws.message.Block; import org.apache.axis2.jaxws.message.Message; @@ -212,9 +213,9 @@ public class LogicalMessageImpl implements LogicalMessage { // DOMSource so that the handler programmer can read the data // multiple times and (as opposed to using a StreamSource) and // they can more easily access the data in DOM form. - DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); - dbf.setNamespaceAware(true); - + DocumentBuilderFactory dbf = + XMLUtils.newSecureDocumentBuilderFactory(); + DocumentBuilder db = dbf.newDocumentBuilder(); Document dom = db.parse(bais); payloads.HANDLER_PAYLOAD = new DOMSource(dom); diff --git a/modules/kernel/src/org/apache/axis2/deployment/resolver/AARBasedWSDLLocator.java b/modules/kernel/src/org/apache/axis2/deployment/resolver/AARBasedWSDLLocator.java index c3e4b367ad..e76b539fff 100644 --- a/modules/kernel/src/org/apache/axis2/deployment/resolver/AARBasedWSDLLocator.java +++ b/modules/kernel/src/org/apache/axis2/deployment/resolver/AARBasedWSDLLocator.java @@ -77,7 +77,14 @@ public class AARBasedWSDLLocator extends DefaultURIResolver implements WSDLLocat if (loc.regionMatches(true, 0, "http:", 0, 5) || loc.regionMatches(true, 0, "https:", 0, 6) || loc.regionMatches(true, 0, "ftp:", 0, 4) - || loc.regionMatches(true, 0, "jar:", 0, 4)) { + || loc.regionMatches(true, 0, "jar:", 0, 4) + // file: as well. The threat model says these resolvers block it, + // and an absolute file: import would read the server's own + // filesystem rather than the archive these resolvers exist to read + // from. Only AARBasedWSDLLocator counts file: as absolute, so only + // there did one reach this branch and fall through to the parent + // resolver; the rest are covered so the guard reads the same way. + || loc.regionMatches(true, 0, "file:", 0, 5)) { throw new RuntimeException( "Remote WSDL import blocked: " + loc); } diff --git a/modules/kernel/src/org/apache/axis2/deployment/resolver/AARFileBasedURIResolver.java b/modules/kernel/src/org/apache/axis2/deployment/resolver/AARFileBasedURIResolver.java index 801a33a97b..e4136b0431 100644 --- a/modules/kernel/src/org/apache/axis2/deployment/resolver/AARFileBasedURIResolver.java +++ b/modules/kernel/src/org/apache/axis2/deployment/resolver/AARFileBasedURIResolver.java @@ -64,7 +64,14 @@ public class AARFileBasedURIResolver extends DefaultURIResolver { if (loc.regionMatches(true, 0, "http:", 0, 5) || loc.regionMatches(true, 0, "https:", 0, 6) || loc.regionMatches(true, 0, "ftp:", 0, 4) - || loc.regionMatches(true, 0, "jar:", 0, 4)) { + || loc.regionMatches(true, 0, "jar:", 0, 4) + // file: as well. The threat model says these resolvers block it, + // and an absolute file: import would read the server's own + // filesystem rather than the archive these resolvers exist to read + // from. Only AARBasedWSDLLocator counts file: as absolute, so only + // there did one reach this branch and fall through to the parent + // resolver; the rest are covered so the guard reads the same way. + || loc.regionMatches(true, 0, "file:", 0, 5)) { log.warn("Blocked remote schema resolution in AAR deployment: " + loc); return new InputSource(new java.io.ByteArrayInputStream(new byte[0])); } diff --git a/modules/kernel/src/org/apache/axis2/deployment/resolver/WarBasedWSDLLocator.java b/modules/kernel/src/org/apache/axis2/deployment/resolver/WarBasedWSDLLocator.java index db8c8274e3..ba37dc26f9 100644 --- a/modules/kernel/src/org/apache/axis2/deployment/resolver/WarBasedWSDLLocator.java +++ b/modules/kernel/src/org/apache/axis2/deployment/resolver/WarBasedWSDLLocator.java @@ -64,7 +64,14 @@ public class WarBasedWSDLLocator extends DefaultURIResolver implements WSDLLocat if (loc.regionMatches(true, 0, "http:", 0, 5) || loc.regionMatches(true, 0, "https:", 0, 6) || loc.regionMatches(true, 0, "ftp:", 0, 4) - || loc.regionMatches(true, 0, "jar:", 0, 4)) { + || loc.regionMatches(true, 0, "jar:", 0, 4) + // file: as well. The threat model says these resolvers block it, + // and an absolute file: import would read the server's own + // filesystem rather than the archive these resolvers exist to read + // from. Only AARBasedWSDLLocator counts file: as absolute, so only + // there did one reach this branch and fall through to the parent + // resolver; the rest are covered so the guard reads the same way. + || loc.regionMatches(true, 0, "file:", 0, 5)) { throw new RuntimeException( "Remote WSDL import blocked: " + loc); } diff --git a/modules/kernel/src/org/apache/axis2/deployment/resolver/WarFileBasedURIResolver.java b/modules/kernel/src/org/apache/axis2/deployment/resolver/WarFileBasedURIResolver.java index a19911db64..6298fd87cf 100644 --- a/modules/kernel/src/org/apache/axis2/deployment/resolver/WarFileBasedURIResolver.java +++ b/modules/kernel/src/org/apache/axis2/deployment/resolver/WarFileBasedURIResolver.java @@ -58,7 +58,14 @@ public class WarFileBasedURIResolver extends DefaultURIResolver { if (resolved.regionMatches(true, 0, "http:", 0, 5) || resolved.regionMatches(true, 0, "https:", 0, 6) || resolved.regionMatches(true, 0, "ftp:", 0, 4) - || resolved.regionMatches(true, 0, "jar:", 0, 4)) { + || resolved.regionMatches(true, 0, "jar:", 0, 4) + // file: as well. The threat model says these resolvers block it, + // and an absolute file: import would read the server's own + // filesystem rather than the archive these resolvers exist to read + // from. Only AARBasedWSDLLocator counts file: as absolute, so only + // there did one reach this branch and fall through to the parent + // resolver; the rest are covered so the guard reads the same way. + || resolved.regionMatches(true, 0, "file:", 0, 5)) { log.warn("Blocked remote schema resolution in WAR deployment: " + resolved); return new InputSource(new java.io.ByteArrayInputStream(new byte[0])); } diff --git a/modules/kernel/src/org/apache/axis2/util/XMLUtils.java b/modules/kernel/src/org/apache/axis2/util/XMLUtils.java index 61911463ea..b30ace36c3 100644 --- a/modules/kernel/src/org/apache/axis2/util/XMLUtils.java +++ b/modules/kernel/src/org/apache/axis2/util/XMLUtils.java @@ -554,8 +554,9 @@ public class XMLUtils { element.serialize(baos); ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); + // This file is the one SECURITY.md item 1 cites for parser hardening, and + // this method sat eight lines below the hardened factory without using it. + DocumentBuilderFactory factory = newSecureDocumentBuilderFactory(); return factory.newDocumentBuilder().parse(bais).getDocumentElement(); } diff --git a/modules/kernel/test/org/apache/axis2/deployment/URIResolverTest.java b/modules/kernel/test/org/apache/axis2/deployment/URIResolverTest.java index 29bd5c1e87..ac20ca4f8f 100644 --- a/modules/kernel/test/org/apache/axis2/deployment/URIResolverTest.java +++ b/modules/kernel/test/org/apache/axis2/deployment/URIResolverTest.java @@ -21,6 +21,7 @@ package org.apache.axis2.deployment; import junit.framework.TestCase; +import org.apache.axis2.deployment.resolver.AARBasedWSDLLocator; import org.apache.axis2.deployment.resolver.AARFileBasedURIResolver; import org.apache.axis2.deployment.resolver.WarFileBasedURIResolver; import org.xml.sax.InputSource; @@ -119,4 +120,39 @@ public class URIResolverTest extends TestCase { return isAbsolute(uri); } } + + /** + * The threat model says these resolvers block file: resolution. Only one of the + * four ever let a file: location reach the scheme guard: AARBasedWSDLLocator + * counts file: as absolute, so an absolute file: import passed the guard, fell + * through to the parent resolver and read the server's filesystem instead of the + * archive. That is the one this closes. + */ + public void testAarWsdlLocatorRefusesAFileImport() { + try { + new AARBasedWSDLLocator(null, null, null) + .getImportInputSource("file:/deploy/service.wsdl", "/etc/passwd"); + fail("an absolute file: import must not be resolved from the filesystem"); + } catch (RuntimeException expected) { + assertTrue("should say it was blocked, was: " + expected.getMessage(), + expected.getMessage().contains("blocked")); + } + } + + /** + * The other three reach safety a different way, and it is worth pinning which: + * their isAbsolute does not count file: as absolute, so the location is treated + * as a name to look up inside the archive and never opens a filesystem path. The + * file: clause added to their scheme guard is therefore unreachable today, and + * kept only so the guard reads the same way in all four. + */ + public void testFileLocationsAreLookedUpInTheArchiveNotTheFilesystem() { + WarFileBasedURIResolver war = + new WarFileBasedURIResolver(getClass().getClassLoader()); + InputSource inputSource = war.resolveEntity(null, + "file:///etc/passwd", "file:///etc/passwd"); + assertNotNull(inputSource); + assertNull("nothing may be read from the filesystem", + inputSource.getByteStream()); + } } diff --git a/modules/metadata/src/org/apache/axis2/jaxws/description/builder/JAXWSRIWSDLGenerator.java b/modules/metadata/src/org/apache/axis2/jaxws/description/builder/JAXWSRIWSDLGenerator.java index fcb53a01e9..f23c3e4711 100644 --- a/modules/metadata/src/org/apache/axis2/jaxws/description/builder/JAXWSRIWSDLGenerator.java +++ b/modules/metadata/src/org/apache/axis2/jaxws/description/builder/JAXWSRIWSDLGenerator.java @@ -20,6 +20,7 @@ package org.apache.axis2.jaxws.description.builder; import org.apache.axiom.util.UIDGenerator; +import org.apache.axis2.util.XMLUtils; import org.apache.axis2.AxisFault; import org.apache.axis2.Constants; import org.apache.axis2.dataretrieval.SchemaSupplier; @@ -325,8 +326,7 @@ public class JAXWSRIWSDLGenerator implements SchemaSupplier, WSDLSupplier { HashMap<String, XmlSchema> docMap = new HashMap<String, XmlSchema>(); // Doc factory to read schema files - DocumentBuilderFactory fac = DocumentBuilderFactory.newInstance(); - fac.setNamespaceAware(true); + DocumentBuilderFactory fac = XMLUtils.newSecureDocumentBuilderFactory(); List<File> schemaFiles = getSchemaFiles(localOutputDirectory); for (File schemaFile : schemaFiles) { diff --git a/modules/saaj/src/org/apache/axis2/saaj/SOAPBodyImpl.java b/modules/saaj/src/org/apache/axis2/saaj/SOAPBodyImpl.java index d0b60487cc..567dd45cff 100644 --- a/modules/saaj/src/org/apache/axis2/saaj/SOAPBodyImpl.java +++ b/modules/saaj/src/org/apache/axis2/saaj/SOAPBodyImpl.java @@ -19,6 +19,7 @@ package org.apache.axis2.saaj; +import org.apache.axis2.util.XMLUtils; import org.apache.axiom.om.OMElement; import org.apache.axiom.om.OMNamespace; import org.apache.axiom.om.OMNode; @@ -457,10 +458,11 @@ public class SOAPBodyImpl extends SOAPElementImpl<org.apache.axiom.soap.SOAPBody Document document; - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); try { + // Created inside the try: the hardened factory reports a parser that + // will not accept the restrictions, which this block already handles. + DocumentBuilderFactory factory = XMLUtils.newSecureDocumentBuilderFactory(); DocumentBuilder builder = factory.newDocumentBuilder(); document = builder.newDocument(); Element element = diff --git a/modules/saaj/src/org/apache/axis2/saaj/util/SAAJUtil.java b/modules/saaj/src/org/apache/axis2/saaj/util/SAAJUtil.java index 20ffe23aef..bfef22ce4d 100644 --- a/modules/saaj/src/org/apache/axis2/saaj/util/SAAJUtil.java +++ b/modules/saaj/src/org/apache/axis2/saaj/util/SAAJUtil.java @@ -19,6 +19,7 @@ package org.apache.axis2.saaj.util; +import org.apache.axis2.util.XMLUtils; import org.apache.axiom.blob.Blob; import org.apache.axiom.om.OMAbstractFactory; import org.apache.axiom.om.OMAttachmentAccessor; @@ -128,8 +129,9 @@ public class SAAJUtil { element.serialize(baos); ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); + // SECURITY.md item 1 says every framework parser factory refuses DTDs and + // external entities; this was one of the sites that did not. + DocumentBuilderFactory factory = XMLUtils.newSecureDocumentBuilderFactory(); return factory.newDocumentBuilder().parse(bais).getDocumentElement(); } diff --git a/src/site/markdown/release-notes/2.0.2.md b/src/site/markdown/release-notes/2.0.2.md index 2a2e0f835e..905e9210c1 100644 --- a/src/site/markdown/release-notes/2.0.2.md +++ b/src/site/markdown/release-notes/2.0.2.md @@ -164,6 +164,16 @@ in `SECURITY.md`. much as to SOAP faults. The reason is now generic unless details are enabled. The error is logged with its stack as before. +- **Two threat-model claims made true, and their limits stated.** The document said + every parser factory the framework creates disables DTDs and external entities, and + that the AAR/WAR resolvers block `file:` resolution. Neither held. The SAAJ and + JAX-WS conversions now use the hardened factory (`XMLUtils.toDOM`, + `SAAJUtil.toDOM`, `SOAPBodyImpl`, `LogicalMessageImpl`, `JAXWSRIWSDLGenerator`), and + `file:` is refused by the deployment resolvers -- which mattered in one of the four, + where an absolute `file:` import had been reaching the filesystem. `SECURITY.md` now + also names what is *not* covered: the build-time codegen factories, the deliberate + hbm.xml DTD exception, and the `TransformerFactory` instances. + - **OpenAPI and Swagger UI output.** Request-controlled values are validated and encoded for the context they are written into, the served page carries a Content-Security-Policy with a per-response script nonce, and the published
