From 132fdea659754ef57b7e3eb4b28c957cca06c9c5 Mon Sep 17 00:00:00 2001 From: Sami Mazouz Date: Tue, 10 Jan 2023 12:43:46 +0100 Subject: [PATCH] Merge pull request from GHSA-22m9-m3ww-53h3 * fix: check post visibility when mentioning Signed-off-by: Sami Mazouz * fix: `mentionsPosts` include is not used and leaks private posts Signed-off-by: Sami Mazouz * chre: use `PostRepository` Signed-off-by: Sami Mazouz Signed-off-by: Sami Mazouz --- extensions/mentions/composer.json | 2 +- extensions/mentions/extend.php | 2 -- extensions/mentions/src/ConfigureMentions.php | 11 ++++-- .../integration/api/PostMentionsTest.php | 35 +++++++++++++++++++ framework/core/src/Formatter/Formatter.php | 7 ++++ framework/core/src/Post/PostRepository.php | 2 +- 6 files changed, 52 insertions(+), 7 deletions(-) diff --git a/extensions/mentions/composer.json b/extensions/mentions/composer.json index 04435e0ce..ff510ef85 100644 --- a/extensions/mentions/composer.json +++ b/extensions/mentions/composer.json @@ -19,7 +19,7 @@ } ], "require": { - "flarum/core": "^1.6" + "flarum/core": "^1.6.3" }, "autoload": { "psr-4": { diff --git a/extensions/mentions/extend.php b/extensions/mentions/extend.php index 794503693..21efec757 100644 --- a/extensions/mentions/extend.php +++ b/extensions/mentions/extend.php @@ -91,11 +91,9 @@ return [ ]), (new Extend\ApiController(Controller\CreatePostController::class)) - ->addInclude(['mentionsPosts', 'mentionsPosts.mentionedBy']) ->addOptionalInclude('mentionsGroups'), (new Extend\ApiController(Controller\UpdatePostController::class)) - ->addInclude(['mentionsPosts', 'mentionsPosts.mentionedBy']) ->addOptionalInclude('mentionsGroups'), (new Extend\ApiController(Controller\AbstractSerializeController::class)) diff --git a/extensions/mentions/src/ConfigureMentions.php b/extensions/mentions/src/ConfigureMentions.php index ef7780755..5b4c7ac71 100644 --- a/extensions/mentions/src/ConfigureMentions.php +++ b/extensions/mentions/src/ConfigureMentions.php @@ -12,10 +12,12 @@ namespace Flarum\Mentions; use Flarum\Group\Group; use Flarum\Http\UrlGenerator; use Flarum\Post\CommentPost; +use Flarum\Post\PostRepository; use Flarum\Settings\SettingsRepositoryInterface; use Flarum\User\User; use Illuminate\Support\Str; use s9e\TextFormatter\Configurator; +use s9e\TextFormatter\Parser; class ConfigureMentions { @@ -115,7 +117,8 @@ class ConfigureMentions $tag->filterChain ->prepend([static::class, 'addPostId']) - ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterPostMentions(tag); }'); + ->setJS('function(tag) { return flarum.extensions["flarum-mentions"].filterPostMentions(tag); }') + ->addParameterByName('actor'); $config->Preg->match('/\B@["|“](?((?!"#[a-z]{0,3}[0-9]+).)+)["|”]#p(?[0-9]+)\b/', $tagName); } @@ -124,9 +127,11 @@ class ConfigureMentions * @param $tag * @return bool */ - public static function addPostId($tag) + public static function addPostId($tag, User $actor) { - $post = CommentPost::find($tag->getAttribute('id')); + $post = resolve(PostRepository::class) + ->queryVisibleTo($actor) + ->find($tag->getAttribute('id')); if ($post) { $tag->setAttribute('discussionid', (int) $post->discussion_id); diff --git a/extensions/mentions/tests/integration/api/PostMentionsTest.php b/extensions/mentions/tests/integration/api/PostMentionsTest.php index b0396a3d7..9bb48d8c0 100644 --- a/extensions/mentions/tests/integration/api/PostMentionsTest.php +++ b/extensions/mentions/tests/integration/api/PostMentionsTest.php @@ -38,6 +38,7 @@ class PostMentionsTest extends TestCase ], 'discussions' => [ ['id' => 2, 'title' => __CLASS__, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 3, 'first_post_id' => 4, 'comment_count' => 2], + ['id' => 50, 'title' => __CLASS__, 'is_private' => true, 'created_at' => Carbon::now(), 'last_posted_at' => Carbon::now(), 'user_id' => 3, 'first_post_id' => 4, 'comment_count' => 1], ], 'posts' => [ ['id' => 4, 'number' => 2, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 3, 'type' => 'comment', 'content' => '@tobyuuu#5'], @@ -49,6 +50,9 @@ class PostMentionsTest extends TestCase ['id' => 10, 'number' => 11, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 4, 'type' => 'comment', 'content' => '@"Bad "#p6 User"#p9'], ['id' => 11, 'number' => 12, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 40, 'type' => 'comment', 'content' => '@"Bad "#p6 User"#p9'], ['id' => 12, 'number' => 13, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 4, 'type' => 'comment', 'content' => '@"acme"#p11'], + + // Restricted access + ['id' => 50, 'number' => 1, 'discussion_id' => 50, 'created_at' => Carbon::now(), 'user_id' => 3, 'type' => 'comment', 'content' => 'no'], ], 'post_mentions_post' => [ ['post_id' => 4, 'mentions_post_id' => 5], @@ -128,6 +132,37 @@ class PostMentionsTest extends TestCase $this->assertNotNull(CommentPost::find($response['data']['id'])->mentionsPosts->find(4)); } + /** + * @test + */ + public function cannot_mention_a_post_without_access() + { + $response = $this->send( + $this->request('POST', '/api/posts', [ + 'authenticatedAs' => 1, + 'json' => [ + 'data' => [ + 'attributes' => [ + 'content' => '@"potato"#p50', + ], + 'relationships' => [ + 'discussion' => ['data' => ['id' => 2]], + ], + ], + ], + ]) + ); + + $this->assertEquals(201, $response->getStatusCode()); + + $response = json_decode($response->getBody(), true); + + $this->assertStringContainsString('potato', $response['data']['attributes']['contentHtml']); + $this->assertEquals('@"potato"#p50', $response['data']['attributes']['content']); + $this->assertStringNotContainsString('PostMention', $response['data']['attributes']['contentHtml']); + $this->assertNull(CommentPost::find($response['data']['id'])->mentionsPosts->find(50)); + } + /** * @test */ diff --git a/framework/core/src/Formatter/Formatter.php b/framework/core/src/Formatter/Formatter.php index 2db336cd2..24808a2f8 100644 --- a/framework/core/src/Formatter/Formatter.php +++ b/framework/core/src/Formatter/Formatter.php @@ -91,6 +91,13 @@ class Formatter { $parser = $this->getParser($context); + /* + * Can be injected in tag or attribute filters by calling: + * ->addParameterByName('actor') on the filter. + * See the mentions extension's ConfigureMentions.php for an example. + */ + $parser->registeredVars['actor'] = $user; + foreach ($this->parsingCallbacks as $callback) { $text = $callback($parser, $context, $text, $user); } diff --git a/framework/core/src/Post/PostRepository.php b/framework/core/src/Post/PostRepository.php index 96ccba0b5..5d03ce062 100644 --- a/framework/core/src/Post/PostRepository.php +++ b/framework/core/src/Post/PostRepository.php @@ -29,7 +29,7 @@ class PostRepository * @param User|null $user * @return Builder */ - protected function queryVisibleTo(User $user = null) + public function queryVisibleTo(User $user = null) { $query = $this->query();