Skip to content

Commit bec32f3

Browse files
authored
Align SSRF URL validation with OkHttp parsing (#6321)
* goalx: snapshot before shenyu-analysis * Align SSRF URL validation with the OkHttp request target The Swagger import SSRF guard was parsing URLs with java.net.URL while the actual request path was interpreted by OkHttp. This mismatch let parser-confusion payloads pass validation but resolve to internal hosts during request execution. The fix validates against OkHttp's HttpUrl semantics and adds regression coverage for the known payload. Constraint: Keep validation semantics aligned with the HTTP client actually used for outbound requests Rejected: Add ad-hoc blacklist checks for backslash or @ combinations | brittle and would miss future parser edge cases Confidence: high Scope-risk: narrow Reversibility: clean Directive: Any future SSRF validation must use the same parser and canonicalization rules as the outbound HTTP client Tested: mvn -pl shenyu-admin -Dtest=UrlSecurityUtilsTest,SwaggerImportServiceTest test Not-tested: Full shenyu-admin test suite and live end-to-end Swagger import against a running admin instance * Name parser-confusion SSRF payload in tests This follow-up replaces the repeated parser-confusion exploit string with a single named constant in the SSRF regression tests. Constraint: Limit the cleanup to the dedicated parser-confusion test surface Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep security regression payloads named when reused across multiple assertions Tested: mvn -pl shenyu-admin -Dtest=UrlSecurityUtilsTest,SwaggerImportServiceTest test Not-tested: Full shenyu-admin test suite * Name allowed URL schemes in SSRF validation This follow-up replaces the raw http/https string checks in UrlSecurityUtils with named protocol constants. Constraint: Keep the cleanup local to the SSRF validation utility without changing behavior Confidence: high Scope-risk: narrow Reversibility: clean Directive: Protocol allowlists should use named constants when shared across security-sensitive validation logic Tested: mvn -pl shenyu-admin -Dtest=UrlSecurityUtilsTest,SwaggerImportServiceTest test Not-tested: Full shenyu-admin test suite
1 parent 2c44e7e commit bec32f3

4 files changed

Lines changed: 68 additions & 15 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,6 @@ Thumbs.db
5050

5151
# Private individual user cursor rules
5252
.cursor/rules/_*.mdc
53+
54+
# local worktrees
55+
.worktrees/

shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/UrlSecurityUtils.java

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@
1717

1818
package org.apache.shenyu.admin.utils;
1919

20+
import okhttp3.HttpUrl;
21+
2022
import java.net.InetAddress;
21-
import java.net.MalformedURLException;
22-
import java.net.URL;
2323
import java.net.UnknownHostException;
2424
import java.util.Arrays;
2525
import java.util.HashSet;
@@ -31,6 +31,10 @@
3131
*/
3232
public final class UrlSecurityUtils {
3333

34+
private static final String HTTP_PROTOCOL = "http";
35+
36+
private static final String HTTPS_PROTOCOL = "https";
37+
3438
/**
3539
* Private constructor to prevent instantiation.
3640
*/
@@ -48,21 +52,20 @@ public static void validateUrlForSSRF(final String url) {
4852
throw new IllegalArgumentException("URL cannot be empty");
4953
}
5054

51-
try {
52-
URL parsedUrl = new URL(url);
53-
String protocol = parsedUrl.getProtocol();
54-
55-
// Only allow HTTP and HTTPS protocols
56-
if (!"http".equals(protocol) && !"https".equals(protocol)) {
57-
throw new IllegalArgumentException("Only HTTP and HTTPS protocols are allowed");
58-
}
55+
HttpUrl parsedUrl = HttpUrl.parse(url);
56+
if (Objects.isNull(parsedUrl)) {
57+
throw new IllegalArgumentException("Invalid URL format");
58+
}
5959

60-
// Validate host for SSRF protection
61-
validateHostForSSRF(parsedUrl.getHost(), parsedUrl.getPort());
60+
String protocol = parsedUrl.scheme();
6261

63-
} catch (MalformedURLException e) {
64-
throw new IllegalArgumentException("Invalid URL format: " + e.getMessage());
62+
// Only allow HTTP and HTTPS protocols
63+
if (!HTTP_PROTOCOL.equals(protocol) && !HTTPS_PROTOCOL.equals(protocol)) {
64+
throw new IllegalArgumentException("Only HTTP and HTTPS protocols are allowed");
6565
}
66+
67+
// Validate host for SSRF protection using the same URL parser as request execution.
68+
validateHostForSSRF(parsedUrl.host(), parsedUrl.port());
6669
}
6770

6871
/**

shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SwaggerImportServiceTest.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,15 @@
2626
import org.apache.shenyu.admin.service.manager.DocManager;
2727

2828
import static org.junit.jupiter.api.Assertions.assertFalse;
29+
import static org.mockito.Mockito.verifyNoInteractions;
2930

3031
/**
3132
* Test for SwaggerImportService.
3233
*/
3334
@ExtendWith(MockitoExtension.class)
3435
public class SwaggerImportServiceTest {
36+
37+
private static final String PARSER_CONFUSION_PAYLOAD = "http://127.0.0.1:6666\\@1.1.1.1";
3538

3639
@Mock
3740
private DocManager docManager;
@@ -51,4 +54,12 @@ public void testConnection() {
5154
// Test valid URL format but may fail to connect
5255
assertFalse(service.testConnection("http://invalid.example.com/swagger.json"));
5356
}
54-
}
57+
58+
@Test
59+
public void testConnectionShouldRejectParserConfusionPayloadBeforeRequest() {
60+
SwaggerImportService service = new SwaggerImportServiceImpl(docManager, httpUtils);
61+
62+
assertFalse(service.testConnection(PARSER_CONFUSION_PAYLOAD));
63+
verifyNoInteractions(httpUtils);
64+
}
65+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.shenyu.admin.utils;
19+
20+
import org.junit.jupiter.api.Test;
21+
22+
import static org.junit.jupiter.api.Assertions.assertThrows;
23+
24+
/**
25+
* Test for {@link UrlSecurityUtils}.
26+
*/
27+
public class UrlSecurityUtilsTest {
28+
29+
private static final String PARSER_CONFUSION_PAYLOAD = "http://127.0.0.1:6666\\@1.1.1.1";
30+
31+
@Test
32+
public void validateUrlForSSRFShouldRejectParserConfusionPayload() {
33+
assertThrows(IllegalArgumentException.class,
34+
() -> UrlSecurityUtils.validateUrlForSSRF(PARSER_CONFUSION_PAYLOAD));
35+
}
36+
}

0 commit comments

Comments
 (0)