From 93931f6a7a7e305545c49b0230d7078efe7630e2 Mon Sep 17 00:00:00 2001 From: kl Date: Thu, 8 Oct 2026 14:11:19 +0800 Subject: [PATCH] fix: validate every preview source before applying trust policies --- .../main/java/cn/keking/utils/WebUtils.java | 63 ++++++++++------ .../cn/keking/web/filter/TrustDirFilter.java | 13 +++- .../cn/keking/web/filter/TrustHostFilter.java | 10 ++- .../filter/PreviewSourceSecurityTests.java | 75 +++++++++++++++++++ 4 files changed, 133 insertions(+), 28 deletions(-) create mode 100644 server/src/test/java/cn/keking/web/filter/PreviewSourceSecurityTests.java diff --git a/server/src/main/java/cn/keking/utils/WebUtils.java b/server/src/main/java/cn/keking/utils/WebUtils.java index beaa9a3e..8fe20b3c 100644 --- a/server/src/main/java/cn/keking/utils/WebUtils.java +++ b/server/src/main/java/cn/keking/utils/WebUtils.java @@ -241,32 +241,50 @@ public class WebUtils { * @return url */ public static String getSourceUrl(ServletRequest request) { - String url = request.getParameter("url"); - String urls = request.getParameter("urls"); - String currentUrl = request.getParameter("currentUrl"); - String urlPath = request.getParameter("urlPath"); + List urls = getSourceUrls(request); + return urls.isEmpty() ? null : urls.get(0); + } + + /** Validate every candidate consumed by preview endpoints, including all images. */ + public static List getSourceUrls(ServletRequest request) { + List urls = new ArrayList<>(); String encryption = request.getParameter("encryption"); - if (StringUtils.isNotBlank(url)) { - return decodeUrl(url,encryption); + try { + for (String name : List.of("url", "currentUrl", "urlPath", "urls")) { + String[] values = request.getParameterValues(name); + if (values == null) { + continue; + } + for (String value : values) { + if (StringUtils.isBlank(value)) { + continue; + } + // picturesPreview decodes currentUrl as Base64 independently of encryption. + String decoded = "currentUrl".equals(name) + ? decodeBase64String(value, StandardCharsets.UTF_8) : decodeUrl(value, encryption); + if (decoded == null) { + return List.of(""); + } + if ("urls".equals(name)) { + urls.addAll(Arrays.asList(decoded.split("\\|", -1))); + } else { + urls.add(decoded); + } + } + } + } catch (RuntimeException e) { + return List.of(""); } - if (StringUtils.isNotBlank(currentUrl)) { - return decodeUrl(currentUrl,encryption); - } - if (StringUtils.isNotBlank(urlPath)) { - return decodeUrl(urlPath,encryption); - } - if (StringUtils.isNotBlank(urls)) { - urls = decodeUrl(urls,encryption); - String[] images = urls.split("\\|"); - return images[0]; - } - return null; + return urls; } /** * 判断地址是否正确 * 高 2022/12/17 */ public static boolean isValidUrl(String url) { + if (url == null) { + return false; + } String regStr = "^((https|http|ftp|rtsp|mms|file)://)";//[.?*]表示匹配的就是本身 Pattern pattern = Pattern.compile(regStr); Matcher matcher = pattern.matcher(url); @@ -350,10 +368,13 @@ public class WebUtils { * @return host */ public static String getHost(String urlStr) { + if (urlStr == null) { + return null; + } try { - URL url = new URL(urlStr); - return url.getHost().toLowerCase(); - } catch (MalformedURLException ignored) { + URL url = normalizedURL(urlStr); + return url.getHost().toLowerCase(Locale.ROOT); + } catch (GalimatiasParseException | MalformedURLException | IllegalArgumentException ignored) { } return null; } diff --git a/server/src/main/java/cn/keking/web/filter/TrustDirFilter.java b/server/src/main/java/cn/keking/web/filter/TrustDirFilter.java index f71e9fd3..f4bb5a95 100644 --- a/server/src/main/java/cn/keking/web/filter/TrustDirFilter.java +++ b/server/src/main/java/cn/keking/web/filter/TrustDirFilter.java @@ -13,6 +13,7 @@ import org.springframework.util.FileCopyUtils; import org.springframework.util.StringUtils; import jakarta.servlet.*; +import jakarta.servlet.http.HttpServletResponse; import java.io.File; import java.io.IOException; @@ -20,6 +21,7 @@ import java.net.URL; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.Locale; +import java.util.List; /** * @author : kl (http://kailing.pub) @@ -45,9 +47,12 @@ public class TrustDirFilter implements Filter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - String url = WebUtils.getSourceUrl(request); - if (!allowPreview(url)) { - response.getWriter().write(this.notTrustDirView); + List urls = WebUtils.getSourceUrls(request); + if (urls.isEmpty() || urls.stream().anyMatch(url -> !allowPreview(url))) { + if (response instanceof HttpServletResponse httpResponse) { + httpResponse.setStatus(HttpServletResponse.SC_FORBIDDEN); + } + response.getWriter().write(this.notTrustDirView == null ? "Forbidden" : this.notTrustDirView); response.getWriter().close(); } else { chain.doFilter(request, response); @@ -61,7 +66,7 @@ public class TrustDirFilter implements Filter { private boolean allowPreview(String urlPath) { // 判断URL是否合法 - if (KkFileUtils.isIllegalFileName(urlPath) || !StringUtils.hasText(urlPath) || !WebUtils.isValidUrl(urlPath)) { + if (!StringUtils.hasText(urlPath) || KkFileUtils.isIllegalFileName(urlPath) || !WebUtils.isValidUrl(urlPath)) { return false; } try { diff --git a/server/src/main/java/cn/keking/web/filter/TrustHostFilter.java b/server/src/main/java/cn/keking/web/filter/TrustHostFilter.java index f7547ad5..88cf1784 100644 --- a/server/src/main/java/cn/keking/web/filter/TrustHostFilter.java +++ b/server/src/main/java/cn/keking/web/filter/TrustHostFilter.java @@ -5,6 +5,7 @@ import cn.keking.utils.WebUtils; import java.io.IOException; import java.util.Map; +import java.util.List; import java.util.Locale; import java.util.concurrent.ConcurrentHashMap; import java.nio.charset.StandardCharsets; @@ -48,9 +49,12 @@ public class TrustHostFilter implements Filter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - String url = WebUtils.getSourceUrl(request); - String host = WebUtils.getHost(url); - if (isNotTrustHost(host) || !WebUtils.isValidUrl(url)) { + List urls = WebUtils.getSourceUrls(request); + String rejectedUrl = urls.stream() + .filter(url -> !WebUtils.isValidUrl(url) || isNotTrustHost(WebUtils.getHost(url))) + .findFirst().orElse(null); + if (urls.isEmpty() || rejectedUrl != null) { + String host = rejectedUrl == null ? null : WebUtils.getHost(rejectedUrl); String currentHost = host == null ? "UNKNOWN" : host; if (response instanceof HttpServletResponse httpServletResponse) { httpServletResponse.setStatus(HttpServletResponse.SC_FORBIDDEN); diff --git a/server/src/test/java/cn/keking/web/filter/PreviewSourceSecurityTests.java b/server/src/test/java/cn/keking/web/filter/PreviewSourceSecurityTests.java new file mode 100644 index 00000000..1bc694cc --- /dev/null +++ b/server/src/test/java/cn/keking/web/filter/PreviewSourceSecurityTests.java @@ -0,0 +1,75 @@ +package cn.keking.web.filter; + +import cn.keking.config.ConfigConstants; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.concurrent.atomic.AtomicBoolean; + +import static org.junit.jupiter.api.Assertions.*; + +class PreviewSourceSecurityTests { + private String previousLocalDir; + @BeforeEach void setup() { + previousLocalDir = ConfigConstants.getLocalPreviewDir(); + ConfigConstants.setLocalPreviewDirValue("/allowed-preview-root"); + ConfigConstants.setTrustHostValue("files.example"); + ConfigConstants.setNotTrustHostValue("127.0.0.1"); + } + @AfterEach void restore() { + org.springframework.test.util.ReflectionTestUtils.setField(ConfigConstants.class, "localPreviewDir", previousLocalDir); + ConfigConstants.setTrustHostValue("default"); + ConfigConstants.setNotTrustHostValue("default"); + } + private String encoded(String url) { + return Base64.getEncoder().encodeToString(url.getBytes(StandardCharsets.UTF_8)); + } + private MockHttpServletResponse filter(MockHttpServletRequest request) throws Exception { + MockHttpServletResponse response = new MockHttpServletResponse(); + AtomicBoolean reached = new AtomicBoolean(); + new TrustHostFilter().doFilter(request, response, (req, res) -> reached.set(true)); + assertEquals(response.getStatus() == 200, reached.get()); + return response; + } + @ParameterizedTest + @ValueSource(strings = {"url", "currentUrl", "urls"}) + void allowedDecoysCannotHideBlockedFetchTarget(String decoy) throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/getCorsFile"); + request.setParameter(decoy, encoded("https://files.example/allowed.pdf")); + request.setParameter("urlPath", encoded("http://127.0.0.1/private.pdf")); + assertEquals(403, filter(request).getStatus()); + } + @Test void validatesEveryImageAndEveryRepeatedParameter() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/picturesPreview"); + request.setParameter("urls", encoded("https://files.example/a.jpg|http://127.0.0.1/b.jpg")); + assertEquals(403, filter(request).getStatus()); + request = new MockHttpServletRequest("GET", "/getCorsFile"); + request.setParameter("urlPath", encoded("https://files.example/a.pdf"), encoded("http://127.0.0.1/a.pdf")); + assertEquals(403, filter(request).getStatus()); + } + @Test void preservesAllowedRequestsAndRejectsMissingOrMalformedSources() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/getCorsFile"); + assertEquals(403, filter(request).getStatus()); + request.setParameter("urlPath", "%%%bad"); + assertEquals(403, filter(request).getStatus()); + request.setParameter("urlPath", encoded("https://files.example/a.pdf")); + assertEquals(200, filter(request).getStatus()); + request.setParameter("url", encoded("https://files.example/b.pdf")); + assertEquals(200, filter(request).getStatus()); + } + @Test void directoryFilterAlsoRejectsHiddenFileTargets() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/getCorsFile"); + request.setParameter("url", encoded("https://files.example/a.pdf")); + request.setParameter("urlPath", encoded("file:///outside-preview-root/private.pdf")); + MockHttpServletResponse response = new MockHttpServletResponse(); + new TrustDirFilter().doFilter(request, response, (req, res) -> fail("Unsafe URL reached controller")); + assertEquals(403, response.getStatus()); + } +}