From 8393905c6d988e62a56b0f2b3fa996015f5559db Mon Sep 17 00:00:00 2001 From: Halo Dev Bot <87291978+halo-dev-bot@users.noreply.github.com> Date: Wed, 14 Jun 2023 17:22:14 +0800 Subject: [PATCH] [release-2.5] fix: file path traversal vulnerability in theme and plugin resource APIs (#4075) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is an automated cherry-pick of #4072 /assign ruibaby ```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")); + } +}