Skip to content

Commit 1136233

Browse files
coheigeacoheigea
committed
Refactoring reference resolving
1 parent cc6c705 commit 1136233

6 files changed

Lines changed: 370 additions & 7 deletions

File tree

‎src/main/java/org/apache/xml/security/stax/impl/resourceResolvers/ResolverFilesystem.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@
2020

2121
import java.io.InputStream;
2222
import java.net.URI;
23+
import java.nio.file.Files;
24+
import java.nio.file.Path;
2325

2426
import org.apache.xml.security.exceptions.XMLSecurityException;
2527
import org.apache.xml.security.stax.ext.ResourceResolver;
2628
import org.apache.xml.security.stax.ext.ResourceResolverLookup;
2729
import org.apache.xml.security.stax.ext.stax.XMLSecStartElement;
30+
import org.apache.xml.security.utils.resolver.ResolverUtils;
2831

2932
/**
3033
* Resolver for local filesystem resources. Use the standard java security-manager to
@@ -49,7 +52,11 @@ public ResourceResolverLookup canResolve(String uri, String baseURI) {
4952
if (uri == null) {
5053
return null;
5154
}
52-
if (uri.startsWith("file:") || baseURI != null && baseURI.startsWith("file:")) {
55+
// At least one of uri or baseURI must start with "file:", and
56+
// neither may carry a different explicit scheme (e.g. http:, https:, ftp:).
57+
if ((uri.startsWith("file:") || baseURI != null && baseURI.startsWith("file:"))
58+
&& !ResolverUtils.hasExplicitNonFileScheme(uri)
59+
&& !ResolverUtils.hasExplicitNonFileScheme(baseURI)) {
5360
return this;
5461
}
5562
return null;
@@ -83,7 +90,8 @@ public InputStream getInputStreamFromExternalReference() throws XMLSecurityExcep
8390
if (tmp.getFragment() != null) {
8491
tmp = new URI(tmp.getScheme(), tmp.getSchemeSpecificPart(), null);
8592
}
86-
return tmp.toURL().openStream();
93+
94+
return Files.newInputStream(Path.of(tmp));
8795
} catch (Exception e) {
8896
throw new XMLSecurityException(e);
8997
}

‎src/main/java/org/apache/xml/security/utils/resolver/ResolverUtils.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.xml.security.utils.resolver;
20+
21+
/**
22+
* Shared utility methods for resource resolver implementations.
23+
*/
24+
public final class ResolverUtils {
25+
26+
private ResolverUtils() {
27+
}
28+
29+
/**
30+
* Returns {@code true} if {@code uri} carries an explicit URI scheme other than
31+
* {@code file:}. Relative URIs (no scheme), same-document references (fragment-only),
32+
* and {@code file:} URIs all return {@code false}.
33+
*
34+
* <p>The scheme is recognised according to RFC 3986:
35+
* {@code scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )}.
36+
* A string whose prefix before the first {@code ':'} does not conform to that grammar
37+
* is treated as a relative URI (returns {@code false}).</p>
38+
*
39+
* @param uri the URI string to test; may be {@code null}
40+
* @return {@code true} if {@code uri} has a non-{@code file:} scheme
41+
*/
42+
public static boolean hasExplicitNonFileScheme(String uri) {
43+
if (uri == null || uri.isEmpty()) {
44+
return false;
45+
}
46+
int colon = uri.indexOf(':');
47+
if (colon <= 0) {
48+
return false; // no scheme present — relative URI or fragment
49+
}
50+
// Validate that every character before ':' is a legal scheme character
51+
// (RFC 3986: ALPHA *( ALPHA / DIGIT / "+" / "-" / "." ))
52+
for (int i = 0; i < colon; i++) {
53+
char c = uri.charAt(i);
54+
boolean valid = (i == 0) ? Character.isLetter(c)
55+
: (Character.isLetterOrDigit(c) || c == '+' || c == '-' || c == '.');
56+
if (!valid) {
57+
return false; // not a valid scheme — treat as relative
58+
}
59+
}
60+
return !uri.startsWith("file:");
61+
}
62+
}

‎src/main/java/org/apache/xml/security/utils/resolver/ResourceResolverContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ public boolean isURISafeToResolve() {
6262
return true;
6363
}
6464
if (uriToResolve != null) {
65-
if (uriToResolve.startsWith("file:") || uriToResolve.startsWith("http:")) {
65+
if (uriToResolve.startsWith("file:") || ResolverUtils.hasExplicitNonFileScheme(uriToResolve)) {
6666
return false;
6767
}
6868
if (!uriToResolve.isEmpty() && uriToResolve.charAt(0) != '#' &&
69-
baseUri != null && (baseUri.startsWith("file:") || baseUri.startsWith("http:"))) {
69+
baseUri != null && (baseUri.startsWith("file:") || ResolverUtils.hasExplicitNonFileScheme(baseUri))) {
7070
return false;
7171
}
7272
}

‎src/main/java/org/apache/xml/security/utils/resolver/implementations/ResolverLocalFilesystem.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.xml.security.utils.resolver.ResourceResolverContext;
3030
import org.apache.xml.security.utils.resolver.ResourceResolverException;
3131
import org.apache.xml.security.utils.resolver.ResourceResolverSpi;
32+
import org.apache.xml.security.utils.resolver.ResolverUtils;
3233

3334
/**
3435
* A simple ResourceResolver for requests into the local filesystem.
@@ -64,15 +65,18 @@ public boolean engineCanResolveURI(ResourceResolverContext context) {
6465
return false;
6566
}
6667

67-
if (context.uriToResolve.isEmpty() || context.uriToResolve.charAt(0) == '#' ||
68-
context.uriToResolve.startsWith("http:")) {
68+
if (context.uriToResolve.isEmpty() || context.uriToResolve.charAt(0) == '#') {
6969
return false;
7070
}
7171

7272
try {
7373
LOG.log(Level.DEBUG, "I was asked whether I can resolve {0}", context.uriToResolve);
7474

75-
if (context.uriToResolve.startsWith("file:") || context.baseUri.startsWith("file:")) {
75+
// At least one of uriToResolve or baseUri must start with "file:", and
76+
// neither may carry a different explicit scheme (e.g. http:, https:, ftp:).
77+
if ((context.uriToResolve.startsWith("file:") || context.baseUri.startsWith("file:"))
78+
&& !ResolverUtils.hasExplicitNonFileScheme(context.uriToResolve)
79+
&& !ResolverUtils.hasExplicitNonFileScheme(context.baseUri)) {
7680
LOG.log(Level.DEBUG, "I state that I can resolve {0}", context.uriToResolve);
7781
return true;
7882
}

‎src/test/java/org/apache/xml/security/test/dom/utils/resolver/ResolverLocalFilesystemTest.java‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.xml.security.test.dom.utils.resolver;
20+
21+
import org.apache.xml.security.test.dom.TestUtils;
22+
import org.apache.xml.security.utils.resolver.ResourceResolverContext;
23+
import org.apache.xml.security.utils.resolver.implementations.ResolverLocalFilesystem;
24+
import org.junit.jupiter.api.Test;
25+
import org.w3c.dom.Attr;
26+
import org.w3c.dom.Document;
27+
28+
import static org.junit.jupiter.api.Assertions.assertFalse;
29+
import static org.junit.jupiter.api.Assertions.assertTrue;
30+
31+
/**
32+
* Tests for the scheme-checking logic in ResolverLocalFilesystem.
33+
*
34+
* The fix requires that at least one of uriToResolve / baseUri starts with "file:"
35+
* AND neither carries a different explicit scheme. This prevents a resolver-hijacking
36+
* attack where an https: (or ftp:, etc.) uriToResolve was incorrectly accepted solely
37+
* because the baseUri happened to start with "file:".
38+
*/
39+
class ResolverLocalFilesystemTest {
40+
41+
static {
42+
org.apache.xml.security.Init.init();
43+
}
44+
45+
private static ResourceResolverContext makeContext(String uri, String baseUri) throws Exception {
46+
Document doc = TestUtils.newDocument();
47+
Attr attr = doc.createAttribute("URI");
48+
attr.setValue(uri);
49+
return new ResourceResolverContext(attr, baseUri, false);
50+
}
51+
52+
/**
53+
* Baseline: a plain file: URI with a file: baseUri is accepted (expected behaviour).
54+
*/
55+
@Test
56+
void testFileUriWithFileBaseUriIsAccepted() throws Exception {
57+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
58+
ResourceResolverContext ctx = makeContext("file:///etc/hosts", "file:///var/app/");
59+
assertTrue(resolver.engineCanResolveURI(ctx),
60+
"A file: URI with a file: baseUri should be accepted");
61+
}
62+
63+
/**
64+
* FIX: an https: uriToResolve must be rejected even when baseUri is "file:".
65+
* Previously the resolver incorrectly claimed ownership of https: URIs solely
66+
* because baseUri started with "file:", allowing resolver hijacking.
67+
*/
68+
@Test
69+
void testHttpsUriIsRejectedWhenBaseUriIsFile() throws Exception {
70+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
71+
ResourceResolverContext ctx = makeContext("https://attacker.com/payload", "file:///var/app/");
72+
73+
assertFalse(resolver.engineCanResolveURI(ctx),
74+
"FIX VERIFIED: https: uriToResolve must be rejected even with a file: baseUri");
75+
}
76+
77+
/**
78+
* FIX: any non-file explicit scheme in uriToResolve must be rejected,
79+
* regardless of baseUri.
80+
*/
81+
@Test
82+
void testFtpUriIsRejectedWhenBaseUriIsFile() throws Exception {
83+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
84+
ResourceResolverContext ctx = makeContext("ftp://attacker.com/file.xml", "file:///var/app/");
85+
86+
assertFalse(resolver.engineCanResolveURI(ctx),
87+
"FIX VERIFIED: ftp: uriToResolve must be rejected even with a file: baseUri");
88+
}
89+
90+
/**
91+
* FIX: http: uriToResolve must also be rejected when baseUri is "file:".
92+
*/
93+
@Test
94+
void testHttpUriIsRejectedWhenBaseUriIsFile() throws Exception {
95+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
96+
ResourceResolverContext ctx = makeContext("http://attacker.com/payload", "file:///var/app/");
97+
98+
assertFalse(resolver.engineCanResolveURI(ctx),
99+
"FIX VERIFIED: http: uriToResolve must be rejected even with a file: baseUri");
100+
}
101+
102+
/**
103+
* A relative uriToResolve (no scheme) combined with a file: baseUri is accepted —
104+
* this is the primary legitimate use case for providing a file: baseUri.
105+
*/
106+
@Test
107+
void testRelativeUriWithFileBaseUriIsAccepted() throws Exception {
108+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
109+
ResourceResolverContext ctx = makeContext("subdoc.xml", "file:///var/app/");
110+
111+
assertTrue(resolver.engineCanResolveURI(ctx),
112+
"A relative uriToResolve with a file: baseUri must be accepted");
113+
}
114+
115+
/**
116+
* Sanity check: an https: URI with no baseUri (or a non-file: baseUri) is
117+
* correctly rejected by engineCanResolveURI.
118+
*/
119+
@Test
120+
void testHttpsUriWithNullBaseUriIsRejected() throws Exception {
121+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
122+
ResourceResolverContext ctx = makeContext("https://attacker.com/payload", null);
123+
124+
assertFalse(resolver.engineCanResolveURI(ctx),
125+
"https: URI with no baseUri should be rejected by the filesystem resolver");
126+
}
127+
128+
/**
129+
* Sanity check: an https: URI with an https: baseUri is also correctly rejected.
130+
*/
131+
@Test
132+
void testHttpsUriWithHttpsBaseUriIsRejected() throws Exception {
133+
ResolverLocalFilesystem resolver = new ResolverLocalFilesystem();
134+
ResourceResolverContext ctx = makeContext("https://attacker.com/payload", "https://victim.com/");
135+
136+
assertFalse(resolver.engineCanResolveURI(ctx),
137+
"https: URI with https: baseUri should be rejected by the filesystem resolver");
138+
}
139+
140+
}

0 commit comments

Comments
 (0)