This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch coheigea/default-uri-limits in repository https://gitbox.apache.org/repos/asf/ws-xmlschema.git
commit ba22cf0e224835f1b173dade3f6069f09424beb9 Author: Colm O hEigeartaigh <[email protected]> AuthorDate: Thu Sep 17 07:07:53 2026 +0100 Place default limits on read timeouts + size on remote schemas --- README.txt | 27 ++++ THREAT-MODEL.md | 21 ++- .../schema/resolver/DefaultURIResolver.java | 177 ++++++++++++++++++++- 3 files changed, 215 insertions(+), 10 deletions(-) diff --git a/README.txt b/README.txt index 0cb6324c..4c780598 100644 --- a/README.txt +++ b/README.txt @@ -68,6 +68,31 @@ adjust the per-document limits: as an entity in one - and FEATURE_SECURE_PROCESSING bounds entity expansion by both count and accumulated size. + When the DefaultURIResolver fetches a schema over http or https, the fetch + is bounded: left to the JDK it has no timeout and no size limit, so one + schemaLocation naming a slow or endless host can hold a parsing thread or + its heap indefinitely. The import and resolution limits above bound the + shape of the import graph, not the cost of a single fetch within it. The + following JVM system properties adjust the bounds: + + org.apache.ws.commons.schema.remote.connectTimeoutMillis + Connect timeout for a remote schema fetch. The default is 5000. + + org.apache.ws.commons.schema.remote.readTimeoutMillis + Per-read timeout for a remote schema fetch. The default is 10000. + + org.apache.ws.commons.schema.remote.maxFetchMillis + Maximum total wall-clock time for one remote schema fetch. The + per-read timeout above bounds each blocking read separately, so this + is what stops a host that trickles bytes below that interval. The + default is 30000. + + org.apache.ws.commons.schema.remote.maxBytes + Maximum bytes accepted from one remote schema fetch. The default is + 67108864 (64 MB). + + file: and jar: locations are read as before, without buffering. + The collections returned by the "read-only" accessors on the schema model are, by default, the live internal collections rather than unmodifiable views. To wrap them so that modification throws instead, set: @@ -86,6 +111,8 @@ For example, set a limit with: -Dorg.apache.ws.commons.schema.protectReadOnlyCollections=true + -Dorg.apache.ws.commons.schema.remote.maxFetchMillis=10000 + =================== Security =================== diff --git a/THREAT-MODEL.md b/THREAT-MODEL.md index a5fa6dc4..a55da09b 100644 --- a/THREAT-MODEL.md +++ b/THREAT-MODEL.md @@ -270,6 +270,7 @@ points*: | `org.apache.ws.commons.schema.maxImportDepth` system property | `64` *(documented: `README.txt`)* | operator-tunable per-process limit | maximum import/include resolution depth for one schema read | | `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.remote.connectTimeoutMillis` / `.readTimeoutMillis` / `.maxFetchMillis` / `.maxBytes` system properties | `5000` / `10000` / `30000` / `67108864` *(documented: `README.txt`)* | operator-tunable per-fetch bounds | bound one remote `DefaultURIResolver` fetch in wall-clock time and bytes; without them the JDK opens a `schemaLocation` with no timeout and no size limit, and a single import can hold a thread or its heap indefinitely | | `org.apache.ws.commons.schema.protectReadOnlyCollections` system property | `false` *(documented: `README.txt`, `CollectionFactory.java` lines 37-48)* | in-process convenience, not a trust boundary | when false, the "read-only" model accessors return the **live internal collections**, not unmodifiable views; §7 places the in-process caller outside the attacker model, so this is a correctness guard rather than a security control | | `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 | @@ -573,8 +574,10 @@ defense-in-depth controls: 3. Supplement XMLSchema's import/include depth, per-read resolution, and structural nesting limits with caller-boundary budgets for total imported bytes and fetch rate per top-level parse. -4. Use connect/read timeouts for import fetches and fail closed on - timeout or policy-check errors. +4. Tune, or tighten beyond, the default per-fetch bounds of §5a, and + fail closed on timeout or policy-check errors. The defaults bound a + remote fetch; an aggregate budget across the whole import graph is + still a caller responsibility. 5. Log import-resolution decisions (requested URI, normalized target, allow/deny result, reason) for incident response and triage. 6. Prefer integrity-controlled schema sources (pinned internal mirror @@ -644,11 +647,13 @@ model, the section that licenses the call. reachable from input *(documented: `XmlSchema.java`)*. → `KNOWN-NON-FINDING`. - **"`URLConnection.getInputStream()` without timeout."** True; - XMLSchema does no read-timeout on fetched imports *(maintainer — - §14 Q12)*. The resolver returns a system ID and the JDK opens the - connection, so a timeout cannot be imposed without changing the - resolver's contract. → `BY-DESIGN: property-disclaimed`; - connect/read timeouts are a §10 item 4 caller responsibility. + `DefaultURIResolver` now opens `http`/`https` fetches itself and + bounds them by connect timeout, per-read timeout, total wall-clock + deadline and byte count (see §5a); `file:` and `jar:` locations are + still returned as a system ID for the parser to open. A report that + an unbounded remote fetch holds a thread or its heap is `VALID` if it + shows a bypass of those bounds. Callers wanting tighter budgets, or + bounds on local reads, still install their own resolver. - **"Path traversal via `XmlSchemaCollection.setBaseUri()`."** Caller- supplied trusted string per §6. → `OUT-OF-MODEL: trusted-input`. - **"Schemas in `w3c-testcases/` contain wide-open DTDs."** W3C @@ -730,7 +735,7 @@ A report against XMLSchema receives exactly one of the following: | `OUT-OF-MODEL: unsupported-component` | Lands in `w3c-testcases/`, `*/src/test/`, `etc/`, `xmlschema-bundle-test/`. | §3 items 4, 8 | | `OUT-OF-MODEL: non-default-build` | Only manifests under a §5a configuration the maintainer rules dev/test (e.g. an unsafe custom `URIResolver`). | §5a | | `OUT-OF-MODEL: out-of-layer` | Concerns a *document* validation step delegated to `javax.xml.validation.Validator`, or a WSDL parser upstream. | §3 items 1–3 | -| `BY-DESIGN: property-disclaimed` | Concerns a §9 property the project explicitly does not provide (no SSRF defense, no guarantee of default DTD acceptance, no schema-size, imported-byte, or fetch-rate ceiling). | §9 | +| `BY-DESIGN: property-disclaimed` | Concerns a §9 property the project explicitly does not provide (no SSRF defense, no guarantee that external DTD or entity content is resolved, no aggregate schema-size, imported-byte, or fetch-rate ceiling across a whole import graph). | §9 | | `KNOWN-NON-FINDING` | Matches a §11a recurring false positive. | §11a | | `MODEL-GAP` | Cannot be cleanly routed to any of the above — triggers §12 model revision. | §12 | diff --git a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/resolver/DefaultURIResolver.java b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/resolver/DefaultURIResolver.java index 5a645cf9..06d6f2c5 100644 --- a/xmlschema-core/src/main/java/org/apache/ws/commons/schema/resolver/DefaultURIResolver.java +++ b/xmlschema-core/src/main/java/org/apache/ws/commons/schema/resolver/DefaultURIResolver.java @@ -19,10 +19,16 @@ package org.apache.ws.commons.schema.resolver; import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.HttpURLConnection; import java.net.MalformedURLException; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.net.URLConnection; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -57,6 +63,35 @@ public class DefaultURIResolver implements CollectionURIResolver { private static final Set<String> ALLOWED_SCHEMES = Collections.unmodifiableSet( new HashSet<String>(Arrays.asList("http", "https", "file", "jar"))); + /** + * Bounds on a single network fetch. Left to the JDK, a schema location naming a slow or + * silent host holds the parsing thread for as long as that host keeps the socket open, and + * one that keeps sending holds as much heap as it cares to send. The import depth and + * resolution limits bound the shape of the import graph, not the cost of one fetch within + * it, so they never come into play: a single import is enough. + */ + public static final String CONNECT_TIMEOUT_PROPERTY = + "org.apache.ws.commons.schema.remote.connectTimeoutMillis"; + public static final String READ_TIMEOUT_PROPERTY = + "org.apache.ws.commons.schema.remote.readTimeoutMillis"; + public static final String MAX_FETCH_MILLIS_PROPERTY = + "org.apache.ws.commons.schema.remote.maxFetchMillis"; + public static final String MAX_BYTES_PROPERTY = + "org.apache.ws.commons.schema.remote.maxBytes"; + + private static final long DEFAULT_CONNECT_TIMEOUT_MILLIS = 5L * 1000L; + private static final long DEFAULT_READ_TIMEOUT_MILLIS = 10L * 1000L; + private static final long DEFAULT_MAX_FETCH_MILLIS = 30L * 1000L; + private static final long DEFAULT_MAX_BYTES = 64L * 1024L * 1024L; + + private final long connectTimeoutMillis = + getLongProperty(CONNECT_TIMEOUT_PROPERTY, DEFAULT_CONNECT_TIMEOUT_MILLIS); + private final long readTimeoutMillis = + getLongProperty(READ_TIMEOUT_PROPERTY, DEFAULT_READ_TIMEOUT_MILLIS); + private final long maxFetchMillis = + getLongProperty(MAX_FETCH_MILLIS_PROPERTY, DEFAULT_MAX_FETCH_MILLIS); + private final long maxBytes = getLongProperty(MAX_BYTES_PROPERTY, DEFAULT_MAX_BYTES); + private String collectionBaseURI; /** @@ -101,7 +136,7 @@ public class DefaultURIResolver implements CollectionURIResolver { URL ref = new URL(base, schemaLocation); verifyComposedUrl(remoteBase, originalBaseUri, base, ref, schemaLocation); - return new InputSource(ref.toString()); + return toInputSource(ref, ref.toString()); } catch (MalformedURLException e1) { throw new XmlSchemaException("Unable to resolve the schema location \"" + schemaLocation + "\" against the base URI \"" + baseUri + "\"", e1); @@ -113,7 +148,11 @@ public class DefaultURIResolver implements CollectionURIResolver { // rejects is not thereby harmless. if (isAbsoluteUri(schemaLocation) || extractScheme(schemaLocation) != null) { verifyPermittedLocation(schemaLocation, schemaLocation); - return new InputSource(schemaLocation); + try { + return toInputSource(new URL(schemaLocation), schemaLocation); + } catch (MalformedURLException e) { + return new InputSource(schemaLocation); + } } if (isPlainRelativePath(schemaLocation)) { return new InputSource(schemaLocation); @@ -122,6 +161,140 @@ public class DefaultURIResolver implements CollectionURIResolver { } + /** + * Hands the parser an InputSource for a resolved location. A <code>file:</code> or + * <code>jar:</code> location keeps the system-id-only form: those reads are local, and the + * parser opens them as it always has. A network location gets a byte stream that the parser + * opens the same way it would have, except that it is bounded - left to the JDK the fetch has + * no timeout and no size limit. + * <p> + * The system id is set either way, and the stream is opened lazily on first read, so this + * method performs no I/O: resolving a location stays a pure URL composition, as callers of + * {@link URIResolver#resolveEntity} expect. Schema documents also carry relative + * <code>schemaLocation</code>s resolved against the system id, so a document that arrives as + * bytes still needs its own URL recorded or its own imports cannot be resolved. + * </p> + */ + private InputSource toInputSource(URL url, String systemId) { + InputSource source = new InputSource(systemId); + if (isNetworkScheme(url.getProtocol().toLowerCase(Locale.ENGLISH))) { + source.setByteStream(new BoundedUrlInputStream(url, systemId)); + } + return source; + } + + /** + * A stream over a remote schema document that opens on first read and enforces the per-fetch + * bounds as it goes. The connect and read timeouts bound each blocking operation separately, + * so they are not on their own enough: a host trickling bytes below the read-timeout interval + * resets that timer indefinitely. The total deadline checked on every read is what bounds + * that, and the running byte count bounds a host that simply keeps sending. + */ + private final class BoundedUrlInputStream extends InputStream { + + private final URL url; + private final String systemId; + private InputStream delegate; + private long deadlineNanos; + private long total; + private boolean closed; + + BoundedUrlInputStream(URL url, String systemId) { + this.url = url; + this.systemId = systemId; + } + + private void ensureOpen() throws IOException { + if (delegate != null) { + return; + } + if (closed) { + throw new IOException("The schema location \"" + systemId + "\" is closed."); + } + URLConnection connection = url.openConnection(); + connection.setDoInput(true); + connection.setConnectTimeout(toIntMillis(connectTimeoutMillis)); + connection.setReadTimeout(toIntMillis(readTimeoutMillis)); + if (connection instanceof HttpURLConnection) { + ((HttpURLConnection)connection).setInstanceFollowRedirects(false); + } + deadlineNanos = System.nanoTime() + maxFetchMillis * 1000000L; + // A declared length is a courtesy: it is absent for a chunked response and is in any + // case whatever the host chose to claim. The running count below is the real limit. + if (connection.getContentLengthLong() > maxBytes) { + throw new IOException("The schema location \"" + systemId + + "\" declared a length above the maximum of " + + maxBytes + " bytes."); + } + delegate = connection.getInputStream(); + } + + private void checkDeadline() throws IOException { + if (System.nanoTime() - deadlineNanos >= 0) { + throw new IOException("Fetching the schema location \"" + systemId + + "\" took longer than the maximum of " + + maxFetchMillis + " ms."); + } + } + + private int count(int read) throws IOException { + if (read > 0) { + total += read; + if (total > maxBytes) { + throw new IOException("The schema location \"" + systemId + + "\" returned more than the maximum of " + + maxBytes + " bytes."); + } + } + return read; + } + + public int read() throws IOException { + ensureOpen(); + checkDeadline(); + int value = delegate.read(); + count(value == -1 ? -1 : 1); + return value; + } + + public int read(byte[] buffer, int offset, int length) throws IOException { + ensureOpen(); + checkDeadline(); + return count(delegate.read(buffer, offset, length)); + } + + public void close() throws IOException { + closed = true; + if (delegate != null) { + delegate.close(); + delegate = null; + } + } + } + + private static int toIntMillis(long millis) { + return millis > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int)millis; + } + + private static long getLongProperty(final String name, long defaultValue) { + try { + String value = AccessController.doPrivileged(new PrivilegedAction<String>() { + public String run() { + return System.getProperty(name); + } + }); + if (value != null && value.trim().length() > 0) { + long parsed = Long.parseLong(value.trim()); + if (parsed > 0) { + return parsed; + } + } + } catch (RuntimeException e) { + // fall through to the default + } + return defaultValue; + } + private static void verifyComposedUrl(boolean remoteBase, String originalBaseUri, URL base, URL composed, String schemaLocation) { verifyPermittedLocation(composed.toString(), schemaLocation);
