This is an automated email from the ASF dual-hosted git repository.
coheigea pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ws-xmlschema.git
The following commit(s) were added to refs/heads/master by this push:
new af61c6f4 Place default limits on read timeouts + size on remote
schemas (#152)
af61c6f4 is described below
commit af61c6f4eb2a1ed9413f597248f60564909c26e5
Author: Colm O hEigeartaigh <[email protected]>
AuthorDate: Thu Sep 17 07:51:19 2026 +0100
Place default limits on read timeouts + size on remote schemas (#152)
---
README.txt | 27 ++++
THREAT-MODEL.md | 21 ++-
.../schema/resolver/DefaultURIResolver.java | 177 ++++++++++++++++++++-
.../src/test/java/tests/RemoteFetchBoundsTest.java | 170 ++++++++++++++++++++
4 files changed, 385 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);
diff --git a/xmlschema-core/src/test/java/tests/RemoteFetchBoundsTest.java
b/xmlschema-core/src/test/java/tests/RemoteFetchBoundsTest.java
new file mode 100644
index 00000000..ae38b252
--- /dev/null
+++ b/xmlschema-core/src/test/java/tests/RemoteFetchBoundsTest.java
@@ -0,0 +1,170 @@
+/**
+ * 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.IOException;
+import java.io.OutputStream;
+import java.io.StringReader;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
+import java.nio.charset.StandardCharsets;
+
+import org.apache.ws.commons.schema.XmlSchema;
+import org.apache.ws.commons.schema.XmlSchemaCollection;
+import org.apache.ws.commons.schema.XmlSchemaException;
+import org.apache.ws.commons.schema.resolver.DefaultURIResolver;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.xml.sax.InputSource;
+
+/**
+ * A schema location naming a hostile host must not be able to hold the
parsing thread or its
+ * heap. The import depth and resolution limits bound the shape of the import
graph, not the
+ * cost of one fetch within it, so a single import reaches none of them.
+ */
+public class RemoteFetchBoundsTest extends Assert {
+
+ private ServerSocket server;
+ private volatile boolean running;
+
+ /** Accepts, then sends nothing and never closes. */
+ private static final int MODE_SILENT = 0;
+ /** Sends a byte at a time forever, resetting the per-read timeout on each
one. */
+ private static final int MODE_TRICKLE = 1;
+ /** Sends a well-formed but endless body. */
+ private static final int MODE_FLOOD = 2;
+
+ private void startServer(final int mode) throws IOException {
+ server = new ServerSocket(0, 50, InetAddress.getByName("127.0.0.1"));
+ running = true;
+ Thread thread = new Thread(new Runnable() {
+ public void run() {
+ while (running) {
+ try {
+ Socket socket = server.accept();
+ serve(socket, mode);
+ } catch (IOException e) {
+ return;
+ }
+ }
+ }
+ });
+ thread.setDaemon(true);
+ thread.start();
+ }
+
+ private void serve(Socket socket, int mode) throws IOException {
+ if (mode == MODE_SILENT) {
+ return;
+ }
+ OutputStream out = socket.getOutputStream();
+ out.write("HTTP/1.1 200 OK\r\nContent-Type:
text/xml\r\n\r\n".getBytes(StandardCharsets.UTF_8));
+ out.flush();
+ byte[] chunk = mode == MODE_TRICKLE
+ ? new byte[] {' '}
+ : new byte[64 * 1024];
+ while (running) {
+ try {
+ out.write(chunk);
+ out.flush();
+ if (mode == MODE_TRICKLE) {
+ Thread.sleep(50);
+ }
+ } catch (Exception e) {
+ return;
+ }
+ }
+ }
+
+ private String importing() {
+ return "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\";
targetNamespace=\"urn:a\">"
+ + "<xs:import namespace=\"urn:b\"
schemaLocation=\"http://127.0.0.1:";
+ + server.getLocalPort() + "/b.xsd\"/>"
+ + "</xs:schema>";
+ }
+
+ @After
+ public void stopServer() throws IOException {
+ running = false;
+ if (server != null) {
+ server.close();
+ }
+ }
+
+ @Before
+ public void shortenBounds() {
+ System.setProperty(DefaultURIResolver.CONNECT_TIMEOUT_PROPERTY,
"2000");
+ System.setProperty(DefaultURIResolver.READ_TIMEOUT_PROPERTY, "1000");
+ System.setProperty(DefaultURIResolver.MAX_FETCH_MILLIS_PROPERTY,
"2000");
+ System.setProperty(DefaultURIResolver.MAX_BYTES_PROPERTY, "1048576");
+ }
+
+ @After
+ public void restoreBounds() {
+ System.clearProperty(DefaultURIResolver.CONNECT_TIMEOUT_PROPERTY);
+ System.clearProperty(DefaultURIResolver.READ_TIMEOUT_PROPERTY);
+ System.clearProperty(DefaultURIResolver.MAX_FETCH_MILLIS_PROPERTY);
+ System.clearProperty(DefaultURIResolver.MAX_BYTES_PROPERTY);
+ }
+
+ private void assertRefusedWithin(long millis) throws IOException {
+ long start = System.currentTimeMillis();
+ try {
+ new XmlSchemaCollection().read(new StringReader(importing()));
+ fail("expected the fetch to be refused");
+ } catch (XmlSchemaException expected) {
+ long elapsed = System.currentTimeMillis() - start;
+ assertTrue("refused, but only after " + elapsed + "ms", elapsed <
millis);
+ }
+ }
+
+ @Test(timeout = 60000)
+ public void testSilentHostDoesNotHoldTheThread() throws IOException {
+ startServer(MODE_SILENT);
+ assertRefusedWithin(30000);
+ }
+
+ /** The case a per-read timeout alone does not catch. */
+ @Test(timeout = 60000)
+ public void testTricklingHostIsCutOffAtTheTotalDeadline() throws
IOException {
+ startServer(MODE_TRICKLE);
+ assertRefusedWithin(30000);
+ }
+
+ @Test(timeout = 60000)
+ public void testOversizedResponseIsRefused() throws IOException {
+ startServer(MODE_FLOOD);
+ assertRefusedWithin(30000);
+ }
+
+ /** Local schemas keep the system-id-only path: no buffering, no behaviour
change. */
+ @Test
+ public void testLocalImportStillResolves() throws Exception {
+ XmlSchemaCollection collection = new XmlSchemaCollection();
+ collection.setBaseUri(Resources.TEST_RESOURCES);
+ XmlSchema schema = collection.read(new
InputSource(Resources.asURI("importBase.xsd")));
+ assertNotNull(schema);
+
assertNotNull(collection.schemaForNamespace("http://soapinterop.org/xsd2";));
+ }
+}
