Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prevent GrowiPlugin from being downloaded outside the plugin directory #9712

Merged

Conversation

NaokiHigashi28
Copy link
Contributor

@NaokiHigashi28 NaokiHigashi28 commented Mar 5, 2025

Task

  • #161423プラグインのinstalledPathにおけるパストラバーサル/コード実行についての問題を解決する

Copy link

changeset-bot bot commented Mar 5, 2025

⚠️ No Changeset found

Latest commit: e885dce

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@NaokiHigashi28 NaokiHigashi28 changed the title Fix: Prevent GrowiPlugin from being downloaded outside the plugin directory fix: Prevent GrowiPlugin from being downloaded outside the plugin directory Mar 5, 2025
catch (deleteErr) {
logger.error(deleteErr);
throw new Error(`Failed to delete plugin from GrowiPlugin documents. Original error: ${deleteErr.message}`);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここでは GrowiPlugin のデータは消さなくてもいいかなあ
logger.error に出力して、あとは飲み込めばいいと思う

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応しました。

catch (err) {
logger.error(err);
throw new Error('Failed to constract plugin path');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

回避したいのは deleteFolder

growiPluginsPath が不正なら、deleteFolder は実行せずその旨を logger.warn で出力。GrowiPlugin.deleteOne({ _id: pluginId }); は実行してほしい。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これだと try-catch で catch に入らなかったら GrowiPlugin.deleteOne({ _id: pluginId }); が実行されないのでは?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改めて条件整理

  • joinAndValidatePath が valid でも invalid でも、GrowiPlugin.deleteOne は実行する
  • GrowiPlugin.deleteOne が成功し、かつ (growiPluginsPath && fs.existsSync(growiPluginsPath)) なら、deleteFolder を実行する

}
}
else {
logger.warn(`Plugin path does not exist : ${growiPluginsPath}`);
}
Copy link
Contributor Author

@NaokiHigashi28 NaokiHigashi28 Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

import された plugin は、サーバーの起動時にインストールされるため。import してから、サーバー再起動の間にも、plugin を削除できるようにした。

catch (err) {
logger.error(err);
throw new Error('Failed to constract plugin path');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

これだと try-catch で catch に入らなかったら GrowiPlugin.deleteOne({ _id: pluginId }); が実行されないのでは?

catch (err) {
logger.error(err);
throw new Error('Failed to constract plugin path');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

改めて条件整理

  • joinAndValidatePath が valid でも invalid でも、GrowiPlugin.deleteOne は実行する
  • GrowiPlugin.deleteOne が成功し、かつ (growiPluginsPath && fs.existsSync(growiPluginsPath)) なら、deleteFolder を実行する

@yuki-takei yuki-takei merged commit 0edd954 into master Mar 11, 2025
20 checks passed
@yuki-takei yuki-takei deleted the fix/161421-prevent-installation-outside-the-plugin-directory branch March 11, 2025 11:04
@github-actions github-actions bot mentioned this pull request Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants