Skip to content

Commit 189600d

Browse files
authored
fix(security): prevent SSRF in outbound plugin requests (#1669)
* fix(security): prevent SSRF in outbound plugin requests Validate URLs, resolved addresses, redirects, and download targets across the console, link plugin, and AI tools. Preserve only narrowly scoped internal storage access and add regression coverage for rebinding and parser edge cases. Signed-off-by: yjlu12 <1064690083@qq.com> * chore(console): apply Spotless formatting Signed-off-by: yjlu12 <1064690083@qq.com> --------- Signed-off-by: yjlu12 <1064690083@qq.com>
1 parent aaef2a2 commit 189600d

26 files changed

Lines changed: 1688 additions & 460 deletions

File tree

console/backend/toolkit/src/main/java/com/iflytek/astron/console/toolkit/service/tool/ToolBoxService.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ public ToolBox createTool(ToolBoxDto toolBoxDto) {
171171
} else {
172172
toolBox = new ToolBox();
173173
}
174-
// Validate endpoint URL legality
175-
if (StringUtils.isNotBlank(toolBox.getEndPoint())) {
176-
urlCheckTool.checkUrl(toolBox.getEndPoint());
174+
// Validate the endpoint submitted in this request before it is copied to the entity or sent
175+
// to the tool service. Validating the existing entity would miss new and changed endpoints.
176+
if (StringUtils.isNotBlank(toolBoxDto.getEndPoint())) {
177+
urlCheckTool.checkUrl(toolBoxDto.getEndPoint());
177178
}
178179
toolBoxDto.setVersion("V1.0");
179180
String schemaString = buildToolBox(toolBox, toolBoxDto);

console/backend/toolkit/src/main/java/com/iflytek/astron/console/toolkit/tool/UrlCheckTool.java

Lines changed: 17 additions & 152 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import java.io.IOException;
1414
import java.net.*;
1515
import java.nio.charset.StandardCharsets;
16-
import java.time.Duration;
1716
import java.util.*;
1817
import java.util.regex.Matcher;
1918
import java.util.regex.Pattern;
@@ -28,18 +27,13 @@
2827
* <li>Restricts protocols to HTTP/HTTPS;</li>
2928
* <li>Prohibits user information (user:pass@host format);</li>
3029
* <li>Rejects IPv6 and IPv4-mapped IPv6 (can be relaxed as needed);</li>
31-
* <li>Resolves a bounded redirect chain and performs blacklist/whitelist validation on each
32-
* hop;</li>
30+
* <li>Resolves the submitted hostname and performs blacklist/whitelist validation without making an
31+
* HTTP request;</li>
3332
* <li>Blocks common short link domains;</li>
3433
* <li>Supports IP blacklist, network segment blacklist, and domain whitelist (configuration source:
3534
* ConfigInfo table).</li>
3635
* </ul>
3736
*
38-
* <p>
39-
* Note: External public method signatures remain unchanged, internal implementation enhanced for
40-
* robustness and readability.
41-
* </p>
42-
*
4337
* @author astron-console-toolkit
4438
*/
4539
@Slf4j
@@ -56,72 +50,13 @@ public class UrlCheckTool {
5650
private static final String IP_WHITE_CATEGORY = "IP_WHITE_LIST";
5751

5852
// ===== Other constants =====
59-
private static final int CONNECT_TIMEOUT_MS = (int) Duration.ofSeconds(5).toMillis();
60-
private static final int READ_TIMEOUT_MS = (int) Duration.ofSeconds(5).toMillis();
61-
private static final int MAX_REDIRECTS = 5;
6253
private static final Pattern DOMAIN_PATTERN = Pattern.compile("https?://([^/]+)", Pattern.CASE_INSENSITIVE);
63-
private static final Set<Integer> REDIRECT_STATUS_CODES = Set.of(301, 302, 303, 307, 308);
6454

6555
// Common short link domains
6656
private static final Set<String> SHORT_LINK_DOMAINS = Set.of(
6757
"bit.ly", "tinyurl.com", "t.co", "rebrandly.com", "is.gd", "t.ly",
6858
"monojson.com", "t.cn", "url.cn", "dwz.cn");
6959

70-
/**
71-
* Gets the direct redirected URL without following it automatically.
72-
*
73-
* <p>
74-
* Implementation details: Uses HEAD method only, disables auto-follow, and only retrieves the
75-
* Location header.
76-
* </p>
77-
*
78-
* @param url the original URL to check for redirects
79-
* @return the redirected URL if redirect found, otherwise the original URL
80-
*/
81-
public String getRedirectUrl(String url) {
82-
return getRedirectUrl(url, readCsvConfig(IP_WHITE_CATEGORY));
83-
}
84-
85-
protected String getRedirectUrl(String url, List<String> ipWhiteList) {
86-
if (StringUtils.isBlank(url))
87-
return url;
88-
89-
try {
90-
RedirectLookupResult result = lookupRedirect(url, ipWhiteList);
91-
if (REDIRECT_STATUS_CODES.contains(result.statusCode)
92-
&& StringUtils.isNotBlank(result.location)) {
93-
return new URL(new URL(url), result.location).toString();
94-
}
95-
} catch (IOException e) {
96-
// Use original URL on network exception
97-
log.debug("getRedirectUrl error: {}", e.toString());
98-
}
99-
return url;
100-
}
101-
102-
private RedirectLookupResult lookupRedirect(String url, List<String> ipWhiteList) throws IOException {
103-
HttpURLConnection conn = null;
104-
try {
105-
URL u = toSafeHttpUrl(url);
106-
ensurePublicAddresses(u.getHost(), ipWhiteList);
107-
URLConnection urlConnection = u.openConnection();
108-
if (!(urlConnection instanceof HttpURLConnection httpURLConnection)) {
109-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_HTTP_HTTPS_ONLY);
110-
}
111-
conn = httpURLConnection;
112-
conn.setInstanceFollowRedirects(false);
113-
conn.setConnectTimeout(CONNECT_TIMEOUT_MS);
114-
conn.setReadTimeout(READ_TIMEOUT_MS);
115-
conn.setRequestMethod("HEAD");
116-
int code = conn.getResponseCode();
117-
return new RedirectLookupResult(code, conn.getHeaderField("Location"));
118-
} finally {
119-
if (conn != null) {
120-
conn.disconnect();
121-
}
122-
}
123-
}
124-
12560
/**
12661
* Throws exception if URL host is IPv6 (current policy: disable IPv6). Silently returns on parsing
12762
* exception (doesn't affect main flow).
@@ -162,17 +97,17 @@ public static void IPv4MappedCheck(String url) {
16297
}
16398

16499
/**
165-
* Blacklist/whitelist validation (considering a bounded redirect chain).
100+
* Blacklist/whitelist validation without issuing an outbound request.
166101
* <ol>
167-
* <li>First validate the original URL before any connection;</li>
102+
* <li>Validate the original URL syntax and destination policy;</li>
168103
* <li>Domain in whitelist → allow;</li>
169104
* <li>Resolve A record to get IPv4/IPv6 (this policy focuses on IPv4 validation);</li>
170105
* <li>Hit IP blacklist → reject;</li>
171106
* <li>Hit network segment blacklist (CIDR) → reject;</li>
172-
* <li>Then inspect redirects and validate every redirected URL before proceeding.</li>
173107
* </ol>
174-
* Silently returns on parsing exception (doesn't affect main flow), let upper layer handle
175-
* uniformly.
108+
* The component that actually performs the HTTP request is responsible for disabling redirects and
109+
* binding destination-IP validation to the socket connection. A validator must not probe a
110+
* user-controlled URL because that probe is itself an SSRF sink.
176111
*
177112
* @param url the URL to validate against blacklists and whitelists
178113
* @throws BusinessException if the URL is blacklisted
@@ -184,22 +119,7 @@ public void checkBlackList(String url) {
184119
List<String> domainWhiteList = readCsvConfig(DOMAIN_WHITE_CATEGORY);
185120
List<String> ipWhiteList = readCsvConfig(IP_WHITE_CATEGORY);
186121

187-
String currentUrl = url;
188-
Set<String> visitedUrls = new HashSet<>();
189-
for (int i = 0; i <= MAX_REDIRECTS; i++) {
190-
validateUrlPolicy(
191-
currentUrl, ipBlackList, segmentBlackList, domainWhiteList, ipWhiteList);
192-
if (!visitedUrls.add(currentUrl)) {
193-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL);
194-
}
195-
196-
String redirectUrl = getRedirectUrl(currentUrl, ipWhiteList);
197-
if (currentUrl.equals(redirectUrl)) {
198-
return;
199-
}
200-
currentUrl = redirectUrl;
201-
}
202-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL);
122+
validateUrlPolicy(url, ipBlackList, segmentBlackList, domainWhiteList, ipWhiteList);
203123

204124
} catch (BusinessException e) {
205125
throw e;
@@ -240,11 +160,14 @@ private void validateUrlAgainstBlacklist(String url, List<String> ipBlackList,
240160
if (StringUtils.isBlank(host))
241161
return;
242162

243-
// Whitelist (case insensitive)
163+
// A domain whitelist may bypass configured IP/CIDR deny lists, but never the built-in
164+
// restricted-address policy. Trusted official internal tools use a separate server-side path.
244165
String asciiHost = IDN.toASCII(host).toLowerCase(Locale.ROOT);
166+
boolean domainWhitelisted = false;
245167
for (String white : domainWhiteList) {
246168
if (asciiHost.equalsIgnoreCase(StringUtils.trimToEmpty(white))) {
247-
return;
169+
domainWhitelisted = true;
170+
break;
248171
}
249172
}
250173

@@ -261,6 +184,9 @@ private void validateUrlAgainstBlacklist(String url, List<String> ipBlackList,
261184
if (SsrfValidators.isRestrictedAddress(inet)) {
262185
throw new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL);
263186
}
187+
if (domainWhitelisted) {
188+
continue;
189+
}
264190
String ip = inet.getHostAddress();
265191

266192
// IPv4 blacklist
@@ -408,7 +334,7 @@ public void symbolCheck(String url) {
408334
* <li>Prohibit userInfo/@</li>
409335
* <li>IPv4-mapped / IPv6 rejection</li>
410336
* <li>Short link rejection</li>
411-
* <li>Blacklist/whitelist validation (considering one redirect)</li>
337+
* <li>Blacklist/whitelist validation without probing the endpoint</li>
412338
* </ol>
413339
*
414340
* <p>
@@ -485,65 +411,4 @@ private List<String> readCsvConfig(String category) {
485411
}
486412
}
487413

488-
private record RedirectLookupResult(int statusCode, String location) {}
489-
490-
private boolean isHostInDomainAllowList(String host, List<String> domainWhiteList) {
491-
if (StringUtils.isBlank(host) || domainWhiteList == null || domainWhiteList.isEmpty()) {
492-
return false;
493-
}
494-
String normalizedHost = StringUtils.lowerCase(StringUtils.trim(host), Locale.ROOT);
495-
for (String allowed : domainWhiteList) {
496-
String normalizedAllowed = StringUtils.lowerCase(StringUtils.trimToEmpty(allowed), Locale.ROOT);
497-
// Remove leading dot if present (e.g., ".example.com" -> "example.com")
498-
if (normalizedAllowed.startsWith(".")) {
499-
normalizedAllowed = normalizedAllowed.substring(1);
500-
}
501-
if (normalizedAllowed.isEmpty()) {
502-
continue;
503-
}
504-
if (normalizedHost.equals(normalizedAllowed) || normalizedHost.endsWith("." + normalizedAllowed)) {
505-
return true;
506-
}
507-
}
508-
return false;
509-
}
510-
511-
private URL toSafeHttpUrl(String url) throws IOException {
512-
try {
513-
URI uri = new URI(url);
514-
String scheme = StringUtils.lowerCase(uri.getScheme(), Locale.ROOT);
515-
if (!"http".equals(scheme) && !"https".equals(scheme)) {
516-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_HTTP_HTTPS_ONLY);
517-
}
518-
if (StringUtils.isNotBlank(uri.getUserInfo())) {
519-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL);
520-
}
521-
String host = uri.getHost();
522-
if (StringUtils.isBlank(host)) {
523-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL);
524-
}
525-
String asciiHost = IDN.toASCII(host);
526-
String path = StringUtils.defaultIfBlank(uri.getPath(), "/");
527-
return new URI(
528-
scheme,
529-
null,
530-
asciiHost,
531-
uri.getPort(),
532-
path,
533-
uri.getQuery(),
534-
null).toURL();
535-
} catch (URISyntaxException e) {
536-
throw new IOException("Illegal URL", e);
537-
}
538-
}
539-
540-
private void ensurePublicAddresses(String host, List<String> ipWhiteList) throws UnknownHostException {
541-
boolean ipLiteral = SsrfValidators.isIpLiteral(host);
542-
for (InetAddress address : InetAddress.getAllByName(host)) {
543-
if (!(ipLiteral && SsrfValidators.isAddressMatchedByIpRules(address, ipWhiteList))
544-
&& SsrfValidators.isRestrictedAddress(address)) {
545-
throw new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL);
546-
}
547-
}
548-
}
549414
}

console/backend/toolkit/src/test/java/com/iflytek/astron/console/toolkit/service/tool/ToolBoxServiceDebugToolTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import static org.mockito.Mockito.mockStatic;
3434
import static org.mockito.Mockito.never;
3535
import static org.mockito.Mockito.verify;
36+
import static org.mockito.Mockito.verifyNoInteractions;
3637
import static org.mockito.Mockito.when;
3738

3839
class ToolBoxServiceDebugToolTest {
@@ -72,9 +73,34 @@ void debugToolV2_allowsTrustedOfficialInternalEndpoint() {
7273
assertThat(request.getServer()).isEqualTo(INTERNAL_ENDPOINT);
7374
assertThat(request.getMethod()).isEqualTo("POST");
7475
assertThat(request.getBody().getString("prompt")).isEqualTo("生成一张小狗的图片");
76+
// This client-visible schema flag is informational only and must not grant private-network
77+
// access in core-link.
78+
assertThat(JSONObject.parseObject(request.getOpenapiSchema())
79+
.getJSONObject("info")
80+
.getBoolean("x-is-official")).isFalse();
7581
verify(urlCheckTool, never()).checkUrl(INTERNAL_ENDPOINT);
7682
}
7783

84+
@Test
85+
void createTool_validatesSubmittedEndpointBeforeBuildingOrSending() {
86+
ToolBoxService service = new ToolBoxService();
87+
UrlCheckTool urlCheckTool = mock(UrlCheckTool.class);
88+
ToolServiceCallHandler toolServiceCallHandler = mock(ToolServiceCallHandler.class);
89+
ReflectionTestUtils.setField(service, "urlCheckTool", urlCheckTool);
90+
ReflectionTestUtils.setField(service, "toolServiceCallHandler", toolServiceCallHandler);
91+
ToolBoxDto dto = new ToolBoxDto();
92+
dto.setEndPoint("http://169.254.169.254/latest/meta-data");
93+
doThrow(new BusinessException(ResponseEnum.TOOLBOX_URL_ILLEGAL))
94+
.when(urlCheckTool)
95+
.checkUrl(dto.getEndPoint());
96+
97+
assertThatThrownBy(() -> service.createTool(dto))
98+
.isInstanceOf(BusinessException.class);
99+
100+
verify(urlCheckTool).checkUrl(dto.getEndPoint());
101+
verifyNoInteractions(toolServiceCallHandler);
102+
}
103+
78104
@Test
79105
void debugToolV2_allowsSeededOfficialInternalEndpointWhenOwnerIsNotAdminUid() {
80106
ToolBoxMapper toolBoxMapper = mock(ToolBoxMapper.class);

0 commit comments

Comments
 (0)