Skip to content

Commit 2ab7d2f

Browse files
fix: Perform additional security checks on overrideServerUrl API
1 parent 40f0341 commit 2ab7d2f

4 files changed

Lines changed: 133 additions & 3 deletions

File tree

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package io.appium.java_client.internal;
16+
17+
import org.openqa.selenium.SessionNotCreatedException;
18+
19+
import java.net.InetAddress;
20+
import java.net.URL;
21+
import java.net.UnknownHostException;
22+
23+
/**
24+
* Validates URLs supplied for {@code overrideServerUrl} so that session traffic cannot be
25+
* silently redirected to loopback, link-local (including cloud metadata), wildcard, or multicast
26+
* destinations.
27+
*/
28+
public final class DirectConnectUrlSafety {
29+
30+
private DirectConnectUrlSafety() {
31+
}
32+
33+
/**
34+
* Ensures the given URL's host does not resolve to an address that is unsafe for automatic
35+
* redirection after session creation.
36+
*
37+
* @param url candidate server URL
38+
* @throws SessionNotCreatedException if the host is missing, cannot be resolved, or resolves
39+
* to a disallowed address
40+
*/
41+
public static void requireSafeOverrideTarget(URL url) throws SessionNotCreatedException {
42+
String host = url.getHost();
43+
if (host == null || host.isEmpty()) {
44+
throw new SessionNotCreatedException(
45+
"Refusing to override the server URL: the URL must include a non-empty host.");
46+
}
47+
48+
final InetAddress[] addresses;
49+
try {
50+
addresses = InetAddress.getAllByName(host);
51+
} catch (UnknownHostException e) {
52+
throw new SessionNotCreatedException(
53+
"Refusing to override the server URL: cannot resolve host '" + host + "'.", e);
54+
}
55+
56+
for (InetAddress address : addresses) {
57+
if (isDisallowed(address)) {
58+
throw new SessionNotCreatedException(String.format(
59+
"Refusing to override the server URL: host '%s' resolves to %s, which is not "
60+
+ "allowed (loopback, link-local, unspecified, or multicast address).",
61+
host,
62+
address.getHostAddress()));
63+
}
64+
}
65+
}
66+
67+
private static boolean isDisallowed(InetAddress address) {
68+
return address.isLoopbackAddress()
69+
|| address.isLinkLocalAddress()
70+
|| address.isAnyLocalAddress()
71+
|| address.isMulticastAddress();
72+
}
73+
}

src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import com.google.common.base.Throwables;
2020
import io.appium.java_client.AppiumClientConfig;
21+
import io.appium.java_client.internal.DirectConnectUrlSafety;
2122
import io.appium.java_client.internal.ReflectionHelpers;
2223
import lombok.Getter;
2324
import org.jspecify.annotations.NullMarked;
@@ -155,9 +156,13 @@ protected HttpClient getClient() {
155156
* Override the http client in the HttpCommandExecutor class with a new http client instance with the given URL.
156157
* It uses the same http client factory and client config for the new http client instance
157158
* if the constructor got them.
158-
* @param serverUrl A url to override.
159+
*
160+
* @param serverUrl A url to override. The host must resolve only to routable addresses; loopback,
161+
* link-local (including metadata service ranges), wildcard, and multicast targets
162+
* are rejected.
159163
*/
160164
protected void overrideServerUrl(URL serverUrl) {
165+
DirectConnectUrlSafety.requireSafeOverrideTarget(serverUrl);
161166
HttpClient newClient = getHttpClientFactory().createClient(appiumClientConfig.baseUrl(serverUrl));
162167
setPrivateFieldValue(HttpCommandExecutor.class, "client", newClient);
163168
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package io.appium.java_client.internal;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.openqa.selenium.SessionNotCreatedException;
5+
6+
import java.net.MalformedURLException;
7+
import java.net.URL;
8+
9+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
10+
import static org.junit.jupiter.api.Assertions.assertThrows;
11+
12+
class DirectConnectUrlSafetyTest {
13+
14+
@Test
15+
void allowsDocumentationNetIpLiteral() throws MalformedURLException {
16+
// RFC 5737 TEST-NET-3; parsed locally without DNS
17+
assertDoesNotThrow(() -> DirectConnectUrlSafety.requireSafeOverrideTarget(
18+
new URL("https://203.0.113.1/wd/hub")));
19+
}
20+
21+
@Test
22+
void rejectsIpv4Loopback() throws MalformedURLException {
23+
assertThrows(SessionNotCreatedException.class, () -> DirectConnectUrlSafety.requireSafeOverrideTarget(
24+
new URL("https://127.0.0.1:4443/wd/hub")));
25+
}
26+
27+
@Test
28+
void rejectsLocalhost() throws MalformedURLException {
29+
assertThrows(SessionNotCreatedException.class, () -> DirectConnectUrlSafety.requireSafeOverrideTarget(
30+
new URL("https://localhost:4443/wd/hub")));
31+
}
32+
33+
@Test
34+
void rejectsLinkLocalMetadataIp() throws MalformedURLException {
35+
assertThrows(SessionNotCreatedException.class, () -> DirectConnectUrlSafety.requireSafeOverrideTarget(
36+
new URL("https://169.254.169.254/wd/hub")));
37+
}
38+
39+
@Test
40+
void rejectsIpv6Loopback() throws MalformedURLException {
41+
assertThrows(SessionNotCreatedException.class, () -> DirectConnectUrlSafety.requireSafeOverrideTarget(
42+
new URL("https://[::1]:4443/wd/hub")));
43+
}
44+
}

src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
import io.appium.java_client.AppiumClientConfig;
44
import io.appium.java_client.MobileCommand;
55
import org.junit.jupiter.api.Test;
6+
import org.openqa.selenium.SessionNotCreatedException;
67

78
import java.net.MalformedURLException;
89
import java.net.URL;
910

1011
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
1112
import static org.junit.jupiter.api.Assertions.assertNotNull;
13+
import static org.junit.jupiter.api.Assertions.assertThrows;
1214

1315
class AppiumCommandExecutorTest {
1416
private static final String APPIUM_URL = "https://appium.example.com";
@@ -35,7 +37,13 @@ void getHttpClientFactory() {
3537
}
3638

3739
@Test
38-
void overrideServerUrl() {
39-
assertDoesNotThrow(() -> createExecutor().overrideServerUrl(new URL("https://direct.example.com")));
40+
void overrideServerUrl() throws MalformedURLException {
41+
assertDoesNotThrow(() -> createExecutor().overrideServerUrl(new URL("https://203.0.113.1/wd/hub")));
42+
}
43+
44+
@Test
45+
void overrideServerUrlRejectsLoopbackTarget() throws MalformedURLException {
46+
assertThrows(SessionNotCreatedException.class,
47+
() -> createExecutor().overrideServerUrl(new URL("https://127.0.0.1:4443/wd/hub")));
4048
}
4149
}

0 commit comments

Comments
 (0)