From 997a73d81b1a948475f49f9ec1f5a84341e10ff2 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Wed, 14 Jun 2023 16:36:13 +0800 Subject: [PATCH] fix: file path traversal vulnerability in theme and plugin resource APIs (#4072) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug /area core /milestone 2.7.x #### What this PR does / why we need it: 修复主题和插件静态资源的文件遍历漏洞 漏洞描述： 攻击者可以通过`/plugins/{name}/assets/console/{*resource}` 和 `/themes/{themeName}/assets/{*resource}` 的 resource 参数部分添加特殊字符（如 ../ 或 ..\）来绕过应用程序的访问控制，访问他们没有权限访问的文件或目录。 修复方法： 访问文件之前检查文件路径是否在被限制的目录下，如: resource = /themes/default/templates/../../test 简化路径为 /themes/test 想限制路径在 `/themes/default/templates` 则已经越权拒绝访问 how to test it? 1. 访问例如 `localhost:8090/themes/theme-earth/assets/dist/../../../../../keys/id_rsa` 来检查获取上级目录，上上级目录是否可以访问到，必须只能访问到 themes/assets下的文件即为合理 2. 类似步骤 1 可以尝试`../`， `..\` 来访问 `localhost:8090/plugins/{name}/assets/console/{*resource}`，必须只能访问到插件的 `classpath:console/` 下的文件即为合理 #### Does this PR introduce a user-facing change? ```release-note 修复主题和插件静态资源的路径遍历漏洞 ``` --- .../plugin/resources/BundleResourceUtils.java | 6 +- .../halo/app/theme/ThemeConfiguration.java | 11 ++-- .../resources/BundleResourceUtilsTest.java | 12 +++- .../app/theme/ThemeConfigurationTest.java | 57 +++++++++++++++++++ 4 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 application/src/test/java/run/halo/app/theme/ThemeConfigurationTest.java diff --git a/application/src/main/java/run/halo/app/plugin/resources/BundleResourceUtils.java b/application/src/main/java/run/halo/app/plugin/resources/BundleResourceUtils.java index 4ef135a4d..d81d589e8 100644 --- a/application/src/main/java/run/halo/app/plugin/resources/BundleResourceUtils.java +++ b/application/src/main/java/run/halo/app/plugin/resources/BundleResourceUtils.java @@ -5,6 +5,8 @@ import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.Resource; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; +import run.halo.app.infra.utils.FileUtils; import run.halo.app.infra.utils.PathUtils; import run.halo.app.plugin.HaloPluginManager; import run.halo.app.plugin.PluginConst; @@ -71,7 +73,9 @@ public abstract class BundleResourceUtils { return null; } String path = PathUtils.combinePath(CONSOLE_BUNDLE_LOCATION, bundleName); - Resource resource = resourceLoader.getResource(path); + String simplifyPath = StringUtils.cleanPath(path); + FileUtils.checkDirectoryTraversal("/" + CONSOLE_BUNDLE_LOCATION, simplifyPath); + Resource resource = resourceLoader.getResource(simplifyPath); return resource.exists() ? resource : null; } diff --git a/application/src/main/java/run/halo/app/theme/ThemeConfiguration.java b/application/src/main/java/run/halo/app/theme/ThemeConfiguration.java index 7f38e8091..1a921761b 100644 --- a/application/src/main/java/run/halo/app/theme/ThemeConfiguration.java +++ b/application/src/main/java/run/halo/app/theme/ThemeConfiguration.java @@ -20,6 +20,7 @@ import org.springframework.web.reactive.function.server.ServerResponse; import org.thymeleaf.extras.springsecurity6.dialect.SpringSecurityDialect; import reactor.core.publisher.Mono; import run.halo.app.infra.ThemeRootGetter; +import run.halo.app.infra.utils.FileUtils; import run.halo.app.theme.dialect.HaloSpringSecurityDialect; import run.halo.app.theme.dialect.LinkExpressionObjectDialect; @@ -65,12 +66,14 @@ public class ThemeConfiguration { }); } - private Path getThemeAssetsPath(String themeName, String resource) { - return themeRoot.get() + Path getThemeAssetsPath(String themeName, String resource) { + Path basePath = themeRoot.get() .resolve(themeName) .resolve("templates") - .resolve("assets") - .resolve(resource); + .resolve("assets"); + Path result = basePath.resolve(resource); + FileUtils.checkDirectoryTraversal(basePath, result); + return result; } @Bean diff --git a/application/src/test/java/run/halo/app/plugin/resources/BundleResourceUtilsTest.java b/application/src/test/java/run/halo/app/plugin/resources/BundleResourceUtilsTest.java index c703dd478..7d36a0ced 100644 --- a/application/src/test/java/run/halo/app/plugin/resources/BundleResourceUtilsTest.java +++ b/application/src/test/java/run/halo/app/plugin/resources/BundleResourceUtilsTest.java @@ -1,9 +1,9 @@ package run.halo.app.plugin.resources; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.lenient; -import static org.mockito.Mockito.when; import java.net.MalformedURLException; import java.net.URL; @@ -16,6 +16,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.pf4j.PluginClassLoader; import org.pf4j.PluginWrapper; import org.springframework.core.io.Resource; +import run.halo.app.infra.exception.AccessDeniedException; import run.halo.app.plugin.HaloPluginManager; /** @@ -34,7 +35,7 @@ class BundleResourceUtilsTest { void setUp() throws MalformedURLException { PluginWrapper pluginWrapper = Mockito.mock(PluginWrapper.class); PluginClassLoader pluginClassLoader = Mockito.mock(PluginClassLoader.class); - when(pluginWrapper.getPluginClassLoader()).thenReturn(pluginClassLoader); + lenient().when(pluginWrapper.getPluginClassLoader()).thenReturn(pluginClassLoader); lenient().when(pluginManager.getPlugin(eq("fake-plugin"))).thenReturn(pluginWrapper); lenient().when(pluginClassLoader.getResource(eq("console/main.js"))).thenReturn( @@ -77,5 +78,10 @@ class BundleResourceUtilsTest { jsBundleResource = BundleResourceUtils.getJsBundleResource(pluginManager, "nothing-plugin", "main.js"); assertThat(jsBundleResource).isNull(); + + assertThatThrownBy(() -> { + BundleResourceUtils.getJsBundleResource(pluginManager, "fake-plugin", + "../test/main.js"); + }).isInstanceOf(AccessDeniedException.class); } -} \ No newline at end of file +} diff --git a/application/src/test/java/run/halo/app/theme/ThemeConfigurationTest.java b/application/src/test/java/run/halo/app/theme/ThemeConfigurationTest.java new file mode 100644 index 000000000..8df630d57 --- /dev/null +++ b/application/src/test/java/run/halo/app/theme/ThemeConfigurationTest.java @@ -0,0 +1,57 @@ +package run.halo.app.theme; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.when; + +import java.nio.file.Path; +import java.nio.file.Paths; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import run.halo.app.infra.ThemeRootGetter; +import run.halo.app.infra.exception.AccessDeniedException; + +@ExtendWith(MockitoExtension.class) +class ThemeConfigurationTest { + @Mock + private ThemeRootGetter themeRootGetter; + + @InjectMocks + private ThemeConfiguration themeConfiguration; + + private final Path themeRoot = Paths.get("/tmp/.halo/themes"); + + @BeforeEach + void setUp() { + when(themeRootGetter.get()).thenReturn(themeRoot); + } + + @Test + void themeAssets() { + Path path = themeConfiguration.getThemeAssetsPath("fake-theme", "hello.jpg"); + assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/hello.jpg")); + + path = themeConfiguration.getThemeAssetsPath("fake-theme", "./hello.jpg"); + assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/./hello.jpg")); + + assertThatThrownBy(() -> { + themeConfiguration.getThemeAssetsPath("fake-theme", "../../hello.jpg"); + }).isInstanceOf(AccessDeniedException.class) + .hasMessage( + "403 FORBIDDEN \"Directory traversal detected: /tmp/" + + ".halo/themes/fake-theme/templates/assets/../../hello.jpg\""); + + path = themeConfiguration.getThemeAssetsPath("fake-theme", "%2e%2e/f.jpg"); + assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/%2e%2e/f.jpg")); + + path = themeConfiguration.getThemeAssetsPath("fake-theme", "f/./../p.jpg"); + assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/f/./../p.jpg")); + + path = themeConfiguration.getThemeAssetsPath("fake-theme", "f../p.jpg"); + assertThat(path).isEqualTo(themeRoot.resolve("fake-theme/templates/assets/f../p.jpg")); + } +}