Merge pull request from GHSA-22m9-m3ww-53h3

* fix: check post visibility when mentioning Signed-off-by: Sami Mazouz <sychocouldy@gmail.com> * fix: `mentionsPosts` include is not used and leaks private posts Signed-off-by: Sami Mazouz <sychocouldy@gmail.com> * chre: use `PostRepository` Signed-off-by: Sami Mazouz <sychocouldy@gmail.com> Signed-off-by: Sami Mazouz <sychocouldy@gmail.com>
flarum · Jan 10, 2023 · ab1c868 · ab1c868
1 parent 8a65ad9
commit ab1c868
Show file tree

Hide file tree

Showing 6 changed files with 52 additions and 7 deletions.
diff --git 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
@@ -91,11 +91,9 @@
         ]),
 
     (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
@@ -12,10 +12,12 @@
 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 @@ private function configurePostMentions(Configurator $config)
 
         $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@["|“](?<displayname>((?!"#[a-z]{0,3}[0-9]+).)+)["|”]#p(?<id>[0-9]+)\b/', $tagName);
     }
@@ -124,9 +127,11 @@ private function configurePostMentions(Configurator $config)
      * @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
@@ -38,6 +38,7 @@ protected function setUp(): void
             ],
             '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' => '<r><POSTMENTION displayname="TobyFlarum___" id="5" number="2" discussionid="2" username="toby">@tobyuuu#5</POSTMENTION></r>'],
@@ -49,6 +50,9 @@ protected function setUp(): void
                 ['id' => 10, 'number' => 11, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 4, 'type' => 'comment', 'content' => '<r><POSTMENTION displayname="Bad &quot;#p6 User" id="9" number="10" discussionid="2">@"Bad "#p6 User"#p9</POSTMENTION></r>'],
                 ['id' => 11, 'number' => 12, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 40, 'type' => 'comment', 'content' => '<r><POSTMENTION displayname="Bad &quot;#p6 User" id="9" number="10" discussionid="2">@"Bad "#p6 User"#p9</POSTMENTION></r>'],
                 ['id' => 12, 'number' => 13, 'discussion_id' => 2, 'created_at' => Carbon::now(), 'user_id' => 4, 'type' => 'comment', 'content' => '<r><POSTMENTION displayname="deleted_user" id="11" number="12" discussionid="2">@"acme"#p11</POSTMENTION></r>'],
+
+                // Restricted access
+                ['id' => 50, 'number' => 1, 'discussion_id' => 50, 'created_at' => Carbon::now(), 'user_id' => 3, 'type' => 'comment', 'content' => '<r>no</r>'],
             ],
             'post_mentions_post' => [
                 ['post_id' => 4, 'mentions_post_id' => 5],
@@ -128,6 +132,37 @@ public function mentioning_a_valid_post_with_new_format_works()
         $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
@@ -91,6 +91,13 @@ public function parse($text, $context = null, User $user = null)
     {
         $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
@@ -29,7 +29,7 @@ public function query()
      * @param User|null $user
      * @return Builder<Post>
      */
-    protected function queryVisibleTo(User $user = null)
+    public function queryVisibleTo(User $user = null)
     {
         $query = $this->query();