diff --git a/src/main/java/org/apache/xml/security/stax/impl/resourceResolvers/ResolverFilesystem.java b/src/main/java/org/apache/xml/security/stax/impl/resourceResolvers/ResolverFilesystem.java index 039d7e07e..5b2416a74 100644 --- a/src/main/java/org/apache/xml/security/stax/impl/resourceResolvers/ResolverFilesystem.java +++ b/src/main/java/org/apache/xml/security/stax/impl/resourceResolvers/ResolverFilesystem.java @@ -20,11 +20,14 @@ import java.io.InputStream; import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; import org.apache.xml.security.exceptions.XMLSecurityException; import org.apache.xml.security.stax.ext.ResourceResolver; import org.apache.xml.security.stax.ext.ResourceResolverLookup; import org.apache.xml.security.stax.ext.stax.XMLSecStartElement; +import org.apache.xml.security.utils.resolver.ResolverUtils; /** * Resolver for local filesystem resources. Use the standard java security-manager to @@ -49,7 +52,11 @@ public ResourceResolverLookup canResolve(String uri, String baseURI) { if (uri == null) { return null; } - if (uri.startsWith("file:") || baseURI != null && baseURI.startsWith("file:")) { + // At least one of uri or baseURI must start with "file:", and + // neither may carry a different explicit scheme (e.g. http:, https:, ftp:). + if ((uri.startsWith("file:") || baseURI != null && baseURI.startsWith("file:")) + && !ResolverUtils.hasExplicitNonFileScheme(uri) + && !ResolverUtils.hasExplicitNonFileScheme(baseURI)) { return this; } return null; @@ -83,7 +90,8 @@ public InputStream getInputStreamFromExternalReference() throws XMLSecurityExcep if (tmp.getFragment() != null) { tmp = new URI(tmp.getScheme(), tmp.getSchemeSpecificPart(), null); } - return tmp.toURL().openStream(); + + return Files.newInputStream(Path.of(tmp)); } catch (Exception e) { throw new XMLSecurityException(e); } diff --git a/src/main/java/org/apache/xml/security/utils/resolver/ResolverUtils.java b/src/main/java/org/apache/xml/security/utils/resolver/ResolverUtils.java new file mode 100644 index 000000000..5a11bd999 --- /dev/null +++ b/src/main/java/org/apache/xml/security/utils/resolver/ResolverUtils.java @@ -0,0 +1,46 @@ +/** + * 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 org.apache.xml.security.utils.resolver; + +/** + * Shared utility methods for resource resolver implementations. + */ +public final class ResolverUtils { + + private ResolverUtils() { + } + + /** + * Returns {@code true} if {@code uri} does not start with {@code file:}. + * Returns {@code false} for {@code null}, empty strings, relative URIs (no {@code ':'}), + * and {@code file:} URIs. + * + * @param uri the URI string to test; may be {@code null} + * @return {@code true} if {@code uri} has a non-{@code file:} scheme + */ + public static boolean hasExplicitNonFileScheme(String uri) { + if (uri == null || uri.isEmpty()) { + return false; + } + if (uri.indexOf(':') <= 0) { + return false; // no scheme present — relative URI or fragment + } + return !uri.startsWith("file:"); + } +} diff --git a/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolverContext.java b/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolverContext.java index 4c8ef4390..5244b8b77 100644 --- a/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolverContext.java +++ b/src/main/java/org/apache/xml/security/utils/resolver/ResourceResolverContext.java @@ -62,11 +62,11 @@ public boolean isURISafeToResolve() { return true; } if (uriToResolve != null) { - if (uriToResolve.startsWith("file:") || uriToResolve.startsWith("http:")) { + if (uriToResolve.startsWith("file:") || ResolverUtils.hasExplicitNonFileScheme(uriToResolve)) { return false; } if (!uriToResolve.isEmpty() && uriToResolve.charAt(0) != '#' && - baseUri != null && (baseUri.startsWith("file:") || baseUri.startsWith("http:"))) { + baseUri != null && (baseUri.startsWith("file:") || ResolverUtils.hasExplicitNonFileScheme(baseUri))) { return false; } } diff --git a/src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverLocalFilesystem.java b/src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverLocalFilesystem.java index f59c2983f..1e6c9f810 100644 --- a/src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverLocalFilesystem.java +++ b/src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverLocalFilesystem.java @@ -29,6 +29,7 @@ import org.apache.xml.security.utils.resolver.ResourceResolverContext; import org.apache.xml.security.utils.resolver.ResourceResolverException; import org.apache.xml.security.utils.resolver.ResourceResolverSpi; +import org.apache.xml.security.utils.resolver.ResolverUtils; /** * A simple ResourceResolver for requests into the local filesystem. @@ -64,15 +65,18 @@ public boolean engineCanResolveURI(ResourceResolverContext context) { return false; } - if (context.uriToResolve.isEmpty() || context.uriToResolve.charAt(0) == '#' || - context.uriToResolve.startsWith("http:")) { + if (context.uriToResolve.isEmpty() || context.uriToResolve.charAt(0) == '#') { return false; } try { LOG.log(Level.DEBUG, "I was asked whether I can resolve {0}", context.uriToResolve); - if (context.uriToResolve.startsWith("file:") || context.baseUri.startsWith("file:")) { + // At least one of uriToResolve or baseUri must start with "file:", and + // neither may carry a different explicit scheme (e.g. http:, https:, ftp:). + if ((context.uriToResolve.startsWith("file:") || context.baseUri.startsWith("file:")) + && !ResolverUtils.hasExplicitNonFileScheme(context.uriToResolve) + && !ResolverUtils.hasExplicitNonFileScheme(context.baseUri)) { LOG.log(Level.DEBUG, "I state that I can resolve {0}", context.uriToResolve); return true; } diff --git a/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResolverLocalFilesystemTest.java b/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResolverLocalFilesystemTest.java new file mode 100644 index 000000000..37c7b9242 --- /dev/null +++ b/src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResolverLocalFilesystemTest.java @@ -0,0 +1,140 @@ +/** + * 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 org.apache.xml.security.test.dom.utils.resolver; + +import org.apache.xml.security.test.dom.TestUtils; +import org.apache.xml.security.utils.resolver.ResourceResolverContext; +import org.apache.xml.security.utils.resolver.implementations.ResolverLocalFilesystem; +import org.junit.jupiter.api.Test; +import org.w3c.dom.Attr; +import org.w3c.dom.Document; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests for the scheme-checking logic in ResolverLocalFilesystem. + * + * The fix requires that at least one of uriToResolve / baseUri starts with "file:" + * AND neither carries a different explicit scheme. This prevents a resolver-hijacking + * attack where an https: (or ftp:, etc.) uriToResolve was incorrectly accepted solely + * because the baseUri happened to start with "file:". + */ +class ResolverLocalFilesystemTest { + + static { + org.apache.xml.security.Init.init(); + } + + private static ResourceResolverContext makeContext(String uri, String baseUri) throws Exception { + Document doc = TestUtils.newDocument(); + Attr attr = doc.createAttribute("URI"); + attr.setValue(uri); + return new ResourceResolverContext(attr, baseUri, false); + } + + /** + * Baseline: a plain file: URI with a file: baseUri is accepted (expected behaviour). + */ + @Test + void testFileUriWithFileBaseUriIsAccepted() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("file:///etc/hosts", "file:///var/app/"); + assertTrue(resolver.engineCanResolveURI(ctx), + "A file: URI with a file: baseUri should be accepted"); + } + + /** + * FIX: an https: uriToResolve must be rejected even when baseUri is "file:". + * Previously the resolver incorrectly claimed ownership of https: URIs solely + * because baseUri started with "file:", allowing resolver hijacking. + */ + @Test + void testHttpsUriIsRejectedWhenBaseUriIsFile() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("https://attacker.com/payload", "file:///var/app/"); + + assertFalse(resolver.engineCanResolveURI(ctx), + "FIX VERIFIED: https: uriToResolve must be rejected even with a file: baseUri"); + } + + /** + * FIX: any non-file explicit scheme in uriToResolve must be rejected, + * regardless of baseUri. + */ + @Test + void testFtpUriIsRejectedWhenBaseUriIsFile() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("ftp://attacker.com/file.xml", "file:///var/app/"); + + assertFalse(resolver.engineCanResolveURI(ctx), + "FIX VERIFIED: ftp: uriToResolve must be rejected even with a file: baseUri"); + } + + /** + * FIX: http: uriToResolve must also be rejected when baseUri is "file:". + */ + @Test + void testHttpUriIsRejectedWhenBaseUriIsFile() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("http://attacker.com/payload", "file:///var/app/"); + + assertFalse(resolver.engineCanResolveURI(ctx), + "FIX VERIFIED: http: uriToResolve must be rejected even with a file: baseUri"); + } + + /** + * A relative uriToResolve (no scheme) combined with a file: baseUri is accepted — + * this is the primary legitimate use case for providing a file: baseUri. + */ + @Test + void testRelativeUriWithFileBaseUriIsAccepted() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("subdoc.xml", "file:///var/app/"); + + assertTrue(resolver.engineCanResolveURI(ctx), + "A relative uriToResolve with a file: baseUri must be accepted"); + } + + /** + * Sanity check: an https: URI with no baseUri (or a non-file: baseUri) is + * correctly rejected by engineCanResolveURI. + */ + @Test + void testHttpsUriWithNullBaseUriIsRejected() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("https://attacker.com/payload", null); + + assertFalse(resolver.engineCanResolveURI(ctx), + "https: URI with no baseUri should be rejected by the filesystem resolver"); + } + + /** + * Sanity check: an https: URI with an https: baseUri is also correctly rejected. + */ + @Test + void testHttpsUriWithHttpsBaseUriIsRejected() throws Exception { + ResolverLocalFilesystem resolver = new ResolverLocalFilesystem(); + ResourceResolverContext ctx = makeContext("https://attacker.com/payload", "https://victim.com/"); + + assertFalse(resolver.engineCanResolveURI(ctx), + "https: URI with https: baseUri should be rejected by the filesystem resolver"); + } + +} diff --git a/src/test/java/org/apache/xml/security/test/stax/resourceResolvers/ResolverFilesystemTest.java b/src/test/java/org/apache/xml/security/test/stax/resourceResolvers/ResolverFilesystemTest.java new file mode 100644 index 000000000..b4cb9cfcb --- /dev/null +++ b/src/test/java/org/apache/xml/security/test/stax/resourceResolvers/ResolverFilesystemTest.java @@ -0,0 +1,149 @@ +/** + * 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 org.apache.xml.security.test.stax.resourceResolvers; + +import org.apache.xml.security.stax.impl.resourceResolvers.ResolverFilesystem; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertFalse; + +/** + * Tests for the scheme-checking logic in ResolverFilesystem. + * + * The fix requires that at least one of uri / baseURI starts with "file:" + * AND neither carries a different explicit scheme. This prevents a live-SSRF + * attack where an https: (or http:, ftp:, etc.) uri was incorrectly accepted + * solely because baseURI happened to start with "file:". + */ +class ResolverFilesystemTest { + + static { + org.apache.xml.security.Init.init(); + } + + // ------------------------------------------------------------------------- + // canResolve() — scheme bypass tests + // ------------------------------------------------------------------------- + + /** + * Baseline: a plain file: URI with a file: baseURI is accepted (expected). + */ + @Test + void testFileUriWithFileBaseUriIsAccepted() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNotNull(resolver.canResolve("file:///etc/hosts", "file:///var/app/"), + "A file: URI with a file: baseURI should be accepted"); + } + + /** + * FIX: https: uri must be rejected even when baseURI is file:. + */ + @Test + void testHttpsUriIsRejectedWhenBaseUriIsFile() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNull(resolver.canResolve("https://attacker.com/payload", "file:///var/app/"), + "FIX VERIFIED: https: uri must be rejected even with a file: baseURI"); + } + + /** + * FIX: http: uri must also be rejected when baseURI is file:. + */ + @Test + void testHttpUriIsRejectedWhenBaseUriIsFile() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNull(resolver.canResolve("http://attacker.com/payload", "file:///var/app/"), + "FIX VERIFIED: http: uri must be rejected even with a file: baseURI"); + } + + /** + * FIX: ftp: and other non-file schemes must also be rejected. + */ + @Test + void testFtpUriIsRejectedWhenBaseUriIsFile() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNull(resolver.canResolve("ftp://attacker.com/file.xml", "file:///var/app/"), + "FIX VERIFIED: ftp: uri must be rejected even with a file: baseURI"); + } + + /** + * A relative uri (no scheme) combined with a file: baseURI is accepted — + * this is the primary legitimate use case. + */ + @Test + void testRelativeUriWithFileBaseUriIsAccepted() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNotNull(resolver.canResolve("subdoc.xml", "file:///var/app/"), + "A relative uri with a file: baseURI must be accepted"); + } + + /** + * VULN: URI.resolve() returns an absolute uri unchanged, so the final URL + * opened by getInputStreamFromExternalReference() will be the attacker's + * https: URL — a live SSRF. Demonstrate that resolve() does not anchor + * the result under the file: base. + */ + @Test + void testUriResolveLeavesAbsoluteUriUnchanged() throws Exception { + java.net.URI base = new java.net.URI("file:///var/app/"); + java.net.URI absolute = new java.net.URI("https://attacker.com/payload"); + + java.net.URI resolved = base.resolve(absolute); + + assertFalse("file".equals(resolved.getScheme()), + "VULN CONFIRMED: resolved URI scheme is '" + resolved.getScheme() + + "', not 'file' — toURL().openStream() will make an outbound HTTPS request"); + } + + // ------------------------------------------------------------------------- + // Sanity checks — cases that should correctly be rejected + // ------------------------------------------------------------------------- + + /** + * An https: URI with no baseURI is correctly rejected. + */ + @Test + void testHttpsUriWithNullBaseUriIsRejected() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNull(resolver.canResolve("https://attacker.com/payload", null), + "https: URI with null baseURI should not be accepted"); + } + + /** + * An https: URI with an https: baseURI is correctly rejected. + */ + @Test + void testHttpsUriWithHttpsBaseUriIsRejected() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNull(resolver.canResolve("https://attacker.com/payload", "https://victim.com/"), + "https: URI with https: baseURI should not be accepted"); + } + + /** + * A null URI is correctly rejected. + */ + @Test + void testNullUriIsRejected() { + ResolverFilesystem resolver = new ResolverFilesystem(); + assertNull(resolver.canResolve(null, "file:///var/app/"), + "null URI should always be rejected"); + } + +}