diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageConverter.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageConverter.java index 7456d16c5..ddeb889c1 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageConverter.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageConverter.java @@ -127,6 +127,9 @@ private void validateUrl(URL value, UrlImageFetchPolicy policy) throws IOExcepti if (host == null || host.trim().isEmpty()) { throw new IOException("URL image host is required"); } + if (value.getUserInfo() != null) { + throw new IOException("URL image user info is not allowed"); + } String normalizedHost; try { @@ -134,6 +137,12 @@ private void validateUrl(URL value, UrlImageFetchPolicy policy) throws IOExcepti } catch (IllegalArgumentException e) { throw new IOException("URL image host is invalid", e); } + if (policy.getAllowedHosts().isEmpty()) { + throw new IOException("Remote URL image fetching is disabled"); + } + if (!policy.getAllowedHosts().contains(normalizedHost)) { + throw new IOException("URL image host is not allowlisted"); + } InetAddress[] addresses = InetAddress.getAllByName(normalizedHost); if (addresses.length == 0) { diff --git a/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageFetchPolicy.java b/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageFetchPolicy.java index 6d46eab3f..157f2346f 100644 --- a/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageFetchPolicy.java +++ b/fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/url/UrlImageFetchPolicy.java @@ -20,6 +20,11 @@ package org.apache.fesod.sheet.converters.url; import java.net.IDN; +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -42,6 +47,7 @@ public final class UrlImageFetchPolicy { private static final UrlImageFetchPolicy DEFAULT = builder().build(); + private final Set allowedHosts; private final boolean allowPrivateNetwork; private final Set allowedPrivateHosts; private final List allowedPrivateCidrs; @@ -50,6 +56,7 @@ public final class UrlImageFetchPolicy { private final int maxImageBytes; private UrlImageFetchPolicy(Builder builder) { + this.allowedHosts = Collections.unmodifiableSet(normalizeAllowedHosts(builder.allowedHosts)); this.allowPrivateNetwork = builder.allowPrivateNetwork; this.allowedPrivateHosts = Collections.unmodifiableSet(normalizeHosts(builder.allowedPrivateHosts)); this.allowedPrivateCidrs = Collections.unmodifiableList(new ArrayList<>(builder.allowedPrivateCidrs)); @@ -82,6 +89,9 @@ private static Set normalizeHosts(Collection hosts) { static String normalizeHost(String host) { String normalized = host.trim().toLowerCase(Locale.ROOT); + if (normalized.indexOf(':') >= 0 || normalized.indexOf('[') >= 0 || normalized.indexOf(']') >= 0) { + return normalizeIpv6Host(normalized); + } while (normalized.endsWith(".")) { normalized = normalized.substring(0, normalized.length() - 1); } @@ -91,7 +101,81 @@ static String normalizeHost(String host) { return IDN.toASCII(normalized); } + private static String normalizeIpv6Host(String host) { + boolean startsWithBracket = host.startsWith("["); + boolean endsWithBracket = host.endsWith("]"); + if (startsWithBracket != endsWithBracket) { + throw new IllegalArgumentException("IPv6 host brackets are invalid"); + } + + String literal = startsWithBracket ? host.substring(1, host.length() - 1) : host; + if (literal.isEmpty() || literal.indexOf('[') >= 0 || literal.indexOf(']') >= 0 || literal.indexOf('%') >= 0) { + throw new IllegalArgumentException("IPv6 host is invalid"); + } + + try { + URI uri = new URI("http://[" + literal + "]/"); + if (uri.getHost() == null) { + throw new IllegalArgumentException("IPv6 host is invalid"); + } + InetAddress address = InetAddress.getByName(literal); + if (!(address instanceof Inet6Address)) { + throw new IllegalArgumentException("IPv6 host is invalid"); + } + return address.getHostAddress().toLowerCase(Locale.ROOT); + } catch (URISyntaxException | UnknownHostException e) { + throw new IllegalArgumentException("IPv6 host is invalid", e); + } + } + + private static Set normalizeAllowedHosts(Collection hosts) { + Set result = new HashSet<>(); + for (String host : hosts) { + if (host == null) { + throw new IllegalArgumentException("Allowed host can not be null"); + } + String normalized = normalizeAllowedHost(host); + result.add(normalized); + } + return result; + } + + private static String normalizeAllowedHost(String host) { + String candidate = host.trim(); + if (candidate.isEmpty()) { + throw new IllegalArgumentException("Allowed host can not be blank"); + } + if (candidate.indexOf('*') >= 0) { + throw new IllegalArgumentException("Allowed host wildcards are not supported"); + } + if (candidate.indexOf('/') >= 0 + || candidate.indexOf('\\') >= 0 + || candidate.indexOf('@') >= 0 + || candidate.indexOf('?') >= 0 + || candidate.indexOf('#') >= 0) { + throw new IllegalArgumentException("Allowed host must not contain URL components"); + } + for (int i = 0; i < candidate.length(); i++) { + char character = candidate.charAt(i); + if (Character.isWhitespace(character) + || Character.isSpaceChar(character) + || Character.isISOControl(character)) { + throw new IllegalArgumentException("Allowed host must not contain whitespace or control characters"); + } + } + try { + String normalized = normalizeHost(candidate); + if (normalized.isEmpty()) { + throw new IllegalArgumentException("Allowed host can not be blank"); + } + return normalized; + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Allowed host is invalid", e); + } + } + public static final class Builder { + private Set allowedHosts = Collections.emptySet(); private boolean allowPrivateNetwork; private Set allowedPrivateHosts = Collections.emptySet(); private List allowedPrivateCidrs = Collections.emptyList(); @@ -101,6 +185,21 @@ public static final class Builder { private Builder() {} + /** + * Allows remote image fetching only for the configured exact hosts. + * + * @param allowedHosts exact host names or IP literals without scheme, port, path, or wildcard; URL ports are + * not evaluated + * @return this builder + */ + public Builder allowedHosts(Collection allowedHosts) { + if (allowedHosts == null) { + throw new IllegalArgumentException("Allowed hosts can not be null"); + } + this.allowedHosts = new HashSet<>(allowedHosts); + return this; + } + public Builder allowPrivateNetwork(boolean allowPrivateNetwork) { this.allowPrivateNetwork = allowPrivateNetwork; return this; diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageConverterTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageConverterTest.java index 2de96c4eb..703847c18 100644 --- a/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageConverterTest.java +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageConverterTest.java @@ -86,8 +86,10 @@ void test_rejectNullSchemePolicy() { @Test void test_httpsOnlyPolicyRejectsHttpUrl() throws Exception { URL url = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); - UrlImageConverter.setFetchPolicy( - UrlImageFetchPolicy.builder().allowedSchemes(SchemePolicy.HTTPS).build()); + UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton("127.0.0.1")) + .allowedSchemes(SchemePolicy.HTTPS) + .build()); IOException exception = Assertions.assertThrows(IOException.class, () -> convert(url)); @@ -96,19 +98,47 @@ void test_httpsOnlyPolicyRejectsHttpUrl() throws Exception { } @Test - void test_rejectLoopbackByDefault() throws Exception { + void test_remoteFetchIsDisabledByDefault() throws Exception { + URL url = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); + + IOException exception = Assertions.assertThrows(IOException.class, () -> convert(url)); + + Assertions.assertTrue(exception.getMessage().contains("disabled")); + Assertions.assertEquals(0, requestCount.get()); + } + + @Test + void test_rejectNonAllowlistedHostBeforeConnection() throws Exception { + URL url = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); + UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton("images.example.com")) + .build()); + + IOException exception = Assertions.assertThrows(IOException.class, () -> convert(url)); + + Assertions.assertTrue(exception.getMessage().contains("allowlisted")); + Assertions.assertEquals(0, requestCount.get()); + } + + @Test + void test_privateHostAllowlistDoesNotEnableRemoteFetching() throws Exception { URL url = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); + UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowPrivateNetwork(true) + .allowedPrivateHosts(Collections.singleton("127.0.0.1")) + .build()); IOException exception = Assertions.assertThrows(IOException.class, () -> convert(url)); - Assertions.assertTrue(exception.getMessage().contains("restricted address")); + Assertions.assertTrue(exception.getMessage().contains("disabled")); Assertions.assertEquals(0, requestCount.get()); } @Test - void test_allowPrivateHostWhenExplicitlyAllowlisted() throws Exception { + void test_allowPrivateHostOnAnyPortWhenExplicitlyAllowlisted() throws Exception { URL url = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton("127.0.0.1")) .allowPrivateNetwork(true) .allowedPrivateHosts(Collections.singleton("127.0.0.1")) .build()); @@ -124,6 +154,7 @@ void test_allowPrivateHostWhenExplicitlyAllowlisted() throws Exception { void test_allowPrivateCidrWhenExplicitlyAllowlisted() throws Exception { URL url = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton("127.0.0.1")) .allowPrivateNetwork(true) .allowedPrivateCidrs(Collections.singleton(CidrBlock.parse("127.0.0.0/8"))) .build()); @@ -134,6 +165,18 @@ void test_allowPrivateCidrWhenExplicitlyAllowlisted() throws Exception { PNG_BYTES, cellData.getImageDataList().get(0).getImage()); } + @Test + void test_rejectUrlUserInfoBeforeConnection() throws Exception { + URL serverUrl = startServer(HttpStatus.OK, PNG_BYTES, "image/png"); + URL url = new URL("http://user:password@127.0.0.1:" + serverUrl.getPort() + "/image.png"); + UrlImageConverter.setFetchPolicy(allowLoopbackPolicy()); + + IOException exception = Assertions.assertThrows(IOException.class, () -> convert(url)); + + Assertions.assertTrue(exception.getMessage().contains("user info")); + Assertions.assertEquals(0, requestCount.get()); + } + @Test void test_rejectNonImageResponse() throws Exception { URL url = startServer(HttpStatus.OK, "root:x:0:0".getBytes("UTF-8"), "text/plain"); @@ -145,13 +188,13 @@ void test_rejectNonImageResponse() throws Exception { } @Test - void test_rejectRedirectToNonAllowlistedPrivateHost() throws Exception { + void test_rejectRedirectToNonAllowlistedHost() throws Exception { URL url = startRedirectServer("http://localhost:8080/image.png"); UrlImageConverter.setFetchPolicy(allowLoopbackPolicy()); IOException exception = Assertions.assertThrows(IOException.class, () -> convert(url)); - Assertions.assertTrue(exception.getMessage().contains("restricted address")); + Assertions.assertTrue(exception.getMessage().contains("allowlisted")); Assertions.assertEquals(1, requestCount.get()); } @@ -160,6 +203,7 @@ void test_rejectImageLargerThanPolicyLimit() throws Exception { byte[] body = Arrays.copyOf(PNG_BYTES, PNG_BYTES.length + 20); URL url = startServer(HttpStatus.OK, body, "image/png"); UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton("127.0.0.1")) .allowPrivateNetwork(true) .allowedPrivateHosts(Collections.singleton("127.0.0.1")) .maxImageBytes(PNG_BYTES.length) @@ -172,6 +216,7 @@ void test_rejectImageLargerThanPolicyLimit() throws Exception { private UrlImageFetchPolicy allowLoopbackPolicy() { return UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton("127.0.0.1")) .allowPrivateNetwork(true) .allowedPrivateHosts(Collections.singleton("127.0.0.1")) .build(); diff --git a/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageFetchPolicyTest.java b/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageFetchPolicyTest.java new file mode 100644 index 000000000..50728bcbd --- /dev/null +++ b/fesod-sheet/src/test/java/org/apache/fesod/sheet/converter/UrlImageFetchPolicyTest.java @@ -0,0 +1,96 @@ +/* + * 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.fesod.sheet.converter; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import org.apache.fesod.sheet.converters.url.SchemePolicy; +import org.apache.fesod.sheet.converters.url.UrlImageFetchPolicy; +import org.apache.fesod.sheet.testkit.Tags; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Tests {@link UrlImageFetchPolicy}. + */ +@Tag(Tags.UNIT) +class UrlImageFetchPolicyTest { + + @Test + void test_defaultPolicyDisablesRemoteFetchingAndSupportsHttpConfiguration() { + UrlImageFetchPolicy policy = UrlImageFetchPolicy.defaultPolicy(); + + Assertions.assertTrue(policy.getAllowedHosts().isEmpty()); + Assertions.assertTrue(policy.getAllowedSchemes().containsAll(SchemePolicy.HTTP_OR_HTTPS.getSchemes())); + } + + @Test + void test_normalizeAllowedHostsForExactMatching() { + UrlImageFetchPolicy policy = UrlImageFetchPolicy.builder() + .allowedHosts(Arrays.asList( + " Images.Example.COM. ", + "images.example.com", + "BÜCHER.example", + "IMAGE_SERVICE.internal", + "[2001:db8::1]", + "2001:0DB8:0:0:0:0:0:1", + "::1")) + .build(); + + Assertions.assertEquals( + new HashSet<>(Arrays.asList( + "images.example.com", + "xn--bcher-kva.example", + "image_service.internal", + "2001:db8:0:0:0:0:0:1", + "0:0:0:0:0:0:0:1")), + policy.getAllowedHosts()); + Assertions.assertFalse(policy.getAllowedHosts().contains("cdn.images.example.com")); + Assertions.assertFalse(policy.getAllowedHosts().contains("images.example.com.attacker.test")); + } + + @Test + void test_rejectNullAllowedHosts() { + Assertions.assertThrows(IllegalArgumentException.class, () -> UrlImageFetchPolicy.builder() + .allowedHosts(null)); + Assertions.assertThrows(IllegalArgumentException.class, () -> UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton(null)) + .build()); + } + + @Test + void test_rejectInvalidAllowedHosts() { + for (String host : Arrays.asList( + "", + "*.example.com", + "https://images.example.com", + "user@images.example.com", + "images.example.com:80", + "[::1]:80", + "image service.internal", + "image\tservice.internal")) { + Assertions.assertThrows(IllegalArgumentException.class, () -> UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singleton(host)) + .build()); + } + } +} diff --git a/website/docs/sheet/write/image.md b/website/docs/sheet/write/image.md index ede0a1f76..122254d93 100644 --- a/website/docs/sheet/write/image.md +++ b/website/docs/sheet/write/image.md @@ -188,16 +188,26 @@ Column `A` holds the text and both images; the second image overlaps column `B`. A `URL` field is fetched over the network while the file is written, under the following fetch policies: -- Default allows `http` and `https` only; +- Remote fetching is disabled by default and requires an explicit exact-host allowlist; +- Configured policies support `http` and `https`; +- Host matching is case-insensitive and does not support wildcards or implicit subdomain matching; +- IPv6 literals are accepted with or without brackets; the allowlist matches hosts only and permits any URL port; - Refuses hosts resolving to a loopback, link-local, site-local or otherwise private address; -- Follows at most 3 redirects and reads at most 10 MB; +- Revalidates the scheme and host after every redirect, follows at most 3 redirects, and reads at most 10 MB; - The connect timeout is 1s and the read timeout 5s. +Only allowlist hosts whose DNS and HTTP services you trust. A shared or attacker-controlled host must not be +allowlisted because the host allowlist is the primary security boundary against DNS-rebinding attacks. + A refused fetch fails the write with an `IOException` naming the rule that stopped it: ```shell URL image protocol is not allowed +Remote URL image fetching is disabled + +URL image host is not allowlisted + URL image host resolves to a restricted address URL image request exceeded redirect limit @@ -211,6 +221,8 @@ The policy is global and can be replaced: @Test public void configureUrlImages() { UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singletonList("images.internal")) + .allowedSchemes(SchemePolicy.HTTP_OR_HTTPS) .maxImageBytes(2 * 1024 * 1024) .maxRedirects(1) // allowPrivateNetwork on its own allows nothing - the host or its @@ -225,4 +237,5 @@ public void configureUrlImages() { } ``` -Call `UrlImageConverter.resetFetchPolicy()` to restore the defaults. +The policy is process-wide and should be configured during application startup. Call +`UrlImageConverter.resetFetchPolicy()` to restore the default deny policy. diff --git a/website/i18n/zh-cn/docusaurus-plugin-content-docs/current/sheet/write/image.md b/website/i18n/zh-cn/docusaurus-plugin-content-docs/current/sheet/write/image.md index 831b8bedc..1182c2e17 100644 --- a/website/i18n/zh-cn/docusaurus-plugin-content-docs/current/sheet/write/image.md +++ b/website/i18n/zh-cn/docusaurus-plugin-content-docs/current/sheet/write/image.md @@ -183,16 +183,26 @@ public void imageCellWrite() throws Exception { 写入文件时将通过 `URL` 下载，但内置了如下的一些安全策略： -- 默认只允许 `http` 和 `https`; +- 默认关闭远程下载，只有配置精确 Host 白名单后才会启用; +- 配置后支持 `http` 和 `https`; +- Host 匹配不区分大小写，不支持通配符或隐式子域匹配; +- IPv6 字面量可以带或不带方括号；白名单只匹配 Host，不校验 URL 端口; - 拒绝解析到回环、链路本地、站点本地等私有地址的主机; -- 最多跟随 3 次重定向，最多读取 10 MB; +- 每次重定向后都会重新校验协议和 Host，最多跟随 3 次重定向，最多读取 10 MB; - 连接超时为 1 秒，读取超时为 5 秒。 +白名单只能包含其 DNS 和 HTTP 服务均可信的 Host。不得加入共享或攻击者可控制的 Host，因为 Host +白名单是阻止 DNS-rebinding 攻击的主要安全边界。 + 不满足上述约束时间会下载失败，且抛出自定义明细错误信息的 `IOException` 异常： ```shell URL image protocol is not allowed +Remote URL image fetching is disabled + +URL image host is not allowlisted + URL image host resolves to a restricted address URL image request exceeded redirect limit @@ -206,6 +216,8 @@ URL image data exceeds maximum size @Test public void configureUrlImages() { UrlImageConverter.setFetchPolicy(UrlImageFetchPolicy.builder() + .allowedHosts(Collections.singletonList("images.internal")) + .allowedSchemes(SchemePolicy.HTTP_OR_HTTPS) .maxImageBytes(2 * 1024 * 1024) .maxRedirects(1) // 只设置 allowPrivateNetwork 不会放行任何地址， @@ -220,4 +232,4 @@ public void configureUrlImages() { } ``` -调用 `UrlImageConverter.resetFetchPolicy()` 可以恢复默认值。 +该策略在进程内全局生效，应在应用启动阶段配置。调用 `UrlImageConverter.resetFetchPolicy()` 可以恢复默认拒绝策略。