From 636ec6329a3e214ae67f875a927b04baae5a5878 Mon Sep 17 00:00:00 2001 From: contour <40132326+Contour-D@users.noreply.github.com> Date: Sat, 10 Jun 2023 22:48:14 +0800 Subject: [PATCH 1/4] Fix AttachmentReconciler repeated execution (#4052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug #### What this PR does / why we need it: Fix AttachmentReconciler repeated execution. #### Which issue(s) this PR fixes: Fixes #3746 #### Special notes for your reviewer: The reconile method in run.halo.app.core.extension.reconciler.attachment.AttachmentReconciler will be executed repeatedly, uploading an attachment will be executed twice, because updating the finalizers property will be in onUpdate of run.halo.app.extension.controller.ExtensionWatcher Will request duplicate addition, I provide a fix for you to review #### Does this PR introduce a user-facing change? ```release-note 修复 AttachmentReconciler 重复执行 ``` --- .../attachment/AttachmentReconciler.java | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java b/application/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java index f1ef94fcb7..d525e2ffa2 100644 --- a/application/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java +++ b/application/src/main/java/run/halo/app/core/extension/reconciler/attachment/AttachmentReconciler.java @@ -40,11 +40,7 @@ public Result reconcile(Request request) { client.fetch(Attachment.class, request.name()).ifPresent(attachment -> { // TODO Handle the finalizer if (attachment.getMetadata().getDeletionTimestamp() != null) { - attachmentService.delete(attachment) - .doOnNext(deletedAttachment -> { - removeFinalizer(attachment.getMetadata().getName()); - }) - .blockOptional(); + removeFinalizer(attachment); return; } // add finalizer @@ -89,14 +85,25 @@ void updateStatus(String attachmentName, AttachmentStatus status) { }); } - void removeFinalizer(String attachmentName) { - client.fetch(Attachment.class, attachmentName).ifPresent(attachment -> { - var finalizers = attachment.getMetadata().getFinalizers(); - if (finalizers != null && finalizers.remove(Constant.FINALIZER_NAME)) { - // update it - client.update(attachment); - } - }); + void removeFinalizer(Attachment oldAttachment) { + if (!hasFinalizer(oldAttachment, Constant.FINALIZER_NAME)) { + return; + } + attachmentService.delete(oldAttachment).block(); + client.fetch(Attachment.class, oldAttachment.getMetadata().getName()) + .ifPresent(attachment -> { + var finalizers = attachment.getMetadata().getFinalizers(); + if (hasFinalizer(attachment, Constant.FINALIZER_NAME) + && finalizers.remove(Constant.FINALIZER_NAME)) { + // update it + client.update(attachment); + } + }); + } + + boolean hasFinalizer(Attachment attachment, String finalizer) { + var finalizers = attachment.getMetadata().getFinalizers(); + return finalizers != null && finalizers.contains(finalizer); } void addFinalizerIfNotSet(String attachmentName, Set existingFinalizers) { 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 2/4] 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 4ef135a4d0..d81d589e80 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.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 static Resource getJsBundleResource(HaloPluginManager pluginManager, Stri 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 7f38e80917..1a921761b6 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.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 RouterFunction themeAssets(WebProperties webProperties) { }); } - 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 c703dd478b..7d36a0ced1 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.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 @@ void getJsBundleResource() { 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 0000000000..8df630d57f --- /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")); + } +} From 6d251a7f581b8d13816f6b613a6def57f763378b Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Wed, 14 Jun 2023 18:08:14 +0800 Subject: [PATCH 3/4] refactor: refresh the plugin wrapper when starting the plugin (#4023) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /kind bug /area core /area plugin /milestone 2.6.x #### What this PR does / why we need it: 修复插件重启后 MainClass 对象缓存未清除的问题 how to test it? 下载此插件： [plugin-starter-1.0.0-SNAPSHOT.jar.zip](https://github.com/halo-dev/halo/files/11620847/plugin-starter-1.0.0-SNAPSHOT.jar.zip) 安装并启动插件，会看到类似如下日志： ``` 测试从 [/var/folders/1z/3hlt62691tx63dxx6y0mryw00000gn/T/halo-plugin3709893537121269748.txt] 文件读取内容 插件启动成功！ ``` 修改日志中给出的文件的内容后 reload 插件会看到`插件启动成功！` 后会跟随最新的文件内容则表示 MainClass 是最新的状态没有缓存。 #### Which issue(s) this PR fixes: Fixes #4016 #### Does this PR introduce a user-facing change? ```release-note 修复插件重启后 MainClass 对象缓存未清除的问题 ``` --- .../java/run/halo/app/plugin/BasePlugin.java | 5 +-- .../reconciler/PluginReconciler.java | 5 +-- .../halo/app/plugin/BasePluginFactory.java | 2 +- .../halo/app/plugin/HaloPluginManager.java | 37 ++++++++++++++++++- 4 files changed, 41 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/run/halo/app/plugin/BasePlugin.java b/api/src/main/java/run/halo/app/plugin/BasePlugin.java index ed186f24bd..f513514fec 100644 --- a/api/src/main/java/run/halo/app/plugin/BasePlugin.java +++ b/api/src/main/java/run/halo/app/plugin/BasePlugin.java @@ -2,7 +2,6 @@ import lombok.extern.slf4j.Slf4j; import org.pf4j.Plugin; -import org.pf4j.PluginManager; import org.pf4j.PluginWrapper; /** @@ -15,12 +14,12 @@ @Slf4j public class BasePlugin extends Plugin { + @Deprecated public BasePlugin(PluginWrapper wrapper) { super(wrapper); log.info("Initialized plugin {}", wrapper.getPluginId()); } - private PluginManager getPluginManager() { - return getWrapper().getPluginManager(); + public BasePlugin() { } } diff --git a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java index b8cedd3155..e4cb26bfa4 100644 --- a/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java +++ b/application/src/main/java/run/halo/app/core/extension/reconciler/PluginReconciler.java @@ -289,8 +289,7 @@ void stopAction(String name) { void stateTransition(String name, Function stateAction, PluginState desiredState) { - PluginWrapper pluginWrapper = getPluginWrapper(name); - PluginState currentState = pluginWrapper.getPluginState(); + PluginState currentState = getPluginWrapper(name).getPluginState(); int maxRetries = PluginState.values().length; for (int i = 0; i < maxRetries && currentState != desiredState; i++) { try { @@ -303,7 +302,7 @@ void stateTransition(String name, Function stateAction, break; } // update current state - currentState = pluginWrapper.getPluginState(); + currentState = getPluginWrapper(name).getPluginState(); } catch (Throwable e) { persistenceFailureStatus(name, e); throw e; diff --git a/application/src/main/java/run/halo/app/plugin/BasePluginFactory.java b/application/src/main/java/run/halo/app/plugin/BasePluginFactory.java index 805f7e99fa..c769ab8e29 100644 --- a/application/src/main/java/run/halo/app/plugin/BasePluginFactory.java +++ b/application/src/main/java/run/halo/app/plugin/BasePluginFactory.java @@ -29,7 +29,7 @@ public Plugin create(PluginWrapper pluginWrapper) { "No bean named 'basePlugin' found in the context create default instance"); DefaultListableBeanFactory beanFactory = context.getDefaultListableBeanFactory(); - BasePlugin pluginInstance = new BasePlugin(pluginWrapper); + BasePlugin pluginInstance = new BasePlugin(); beanFactory.registerSingleton(Plugin.class.getName(), pluginInstance); return pluginInstance; } diff --git a/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java b/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java index b748e04c79..7abcb1d5f4 100644 --- a/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java +++ b/application/src/main/java/run/halo/app/plugin/HaloPluginManager.java @@ -229,7 +229,11 @@ public boolean validatePluginVersion(PluginWrapper pluginWrapper) { private PluginState doStartPlugin(String pluginId) { checkPluginId(pluginId); - PluginWrapper pluginWrapper = getPlugin(pluginId); + // refresh plugin to ensure cache object of PluginWrapper.plugin is up-to-date + // see gh-4016 to know why we need this + // TODO if has a better way to do this? + PluginWrapper pluginWrapper = refreshPluginWrapper(pluginId); + checkExtensionFinderReady(pluginWrapper); PluginDescriptor pluginDescriptor = pluginWrapper.getDescriptor(); @@ -423,6 +427,37 @@ private void removePluginComponentsCache(String pluginId) { } } + /** + *

Refresh plugin wrapper by plugin name.

+ * + *

It will be create a new plugin wrapper and replace old plugin wrapper to clean + * {@link PluginWrapper#getPlugin()} cache object.

+ * + * @param pluginName plugin name + * @return refreshed plugin wrapper instance, plugin cache object will be null + * @throws IllegalArgumentException if plugin not found + */ + protected synchronized PluginWrapper refreshPluginWrapper(String pluginName) { + checkPluginId(pluginName); + // get old plugin wrapper + PluginWrapper pluginWrapper = getPlugin(pluginName); + // create new plugin wrapper to replace old plugin wrapper + PluginWrapper refreshed = copyPluginWrapper(pluginWrapper); + this.plugins.put(pluginName, refreshed); + return refreshed; + } + + @NonNull + PluginWrapper copyPluginWrapper(@NonNull PluginWrapper pluginWrapper) { + PluginWrapper refreshed = + createPluginWrapper(pluginWrapper.getDescriptor(), pluginWrapper.getPluginPath(), + pluginWrapper.getPluginClassLoader()); + refreshed.setPluginFactory(getPluginFactory()); + refreshed.setPluginState(pluginWrapper.getPluginState()); + refreshed.setFailedException(pluginWrapper.getFailedException()); + return refreshed; + } + @Override public void destroy() throws Exception { stopPlugins(); From 350e54d42afec1b5f6e574bfd0e3de6434a9525d Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Thu, 15 Jun 2023 22:30:12 +0800 Subject: [PATCH 4/4] chore: bump google guava version to 32.0.1-jre (#4081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /milestone 2.7.x /area core #### What this PR does / why we need it: 升级 Google Guava 版本至 32.0.1-jre Guava [31.1](https://github.com/google/guava/releases/tag/v31.1) 至 [32.0.1](https://github.com/google/guava/releases/tag/v32.0.1) 的变化： 1. 移除了部分 API 的 `@Beta` 注解进入稳定版 2. 关于 `Files.createTempDir` 方法的安全性修复 https://github.com/advisories/GHSA-7g45-4rm6-3mm3 (https://github.com/google/guava/issues/2575) 详情参考：https://github.com/google/guava/releases/tag/v32.0.0 #### Does this PR introduce a user-facing change? ```release-note 升级 Google Guava 版本至 32.0.1-jre ``` --- application/build.gradle | 1 - platform/application/build.gradle | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/application/build.gradle b/application/build.gradle index a9a525cd19..a5d55a7996 100644 --- a/application/build.gradle +++ b/application/build.gradle @@ -51,7 +51,6 @@ ext { base62 = "0.1.3" pf4j = '3.9.0' javaDiffUtils = "4.12" - guava = "31.1-jre" jsoup = '1.15.3' jsonPatch = "1.13" springDocOpenAPI = "2.0.2" diff --git a/platform/application/build.gradle b/platform/application/build.gradle index 9434fd6e26..93a6125526 100644 --- a/platform/application/build.gradle +++ b/platform/application/build.gradle @@ -15,7 +15,7 @@ ext { base62 = "0.1.3" pf4j = '3.9.0' javaDiffUtils = "4.12" - guava = "31.1-jre" + guava = "32.0.1-jre" jsoup = '1.15.3' jsonPatch = "1.13" springDocOpenAPI = "2.1.0"