fix: filter values are not validated (#3795)

extensions/likes/src/Query/LikedByFilter.php

@@ -11,17 +11,20 @@ namespace Flarum\Likes\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class LikedByFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'likedBy';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $likedId = trim($filterValue, '"');
+        $likedId = $this->asInt($filterValue);
 
         $filterState
             ->getQuery()

extensions/lock/src/Query/LockedFilterGambit.php

@@ -32,7 +32,7 @@ class LockedFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'locked';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         $this->constrain($filterState->getQuery(), $negate);
     }

extensions/mentions/src/Filter/MentionedFilter.php

@@ -11,17 +11,20 @@ namespace Flarum\Mentions\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class MentionedFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'mentioned';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $mentionedId = trim($filterValue, '"');
+        $mentionedId = $this->asInt($filterValue);
 
         $filterState
             ->getQuery()

extensions/sticky/src/Query/StickyFilterGambit.php

@@ -32,7 +32,7 @@ class StickyFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'sticky';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, $negate)
+    public function filter(FilterState $filterState, $filterValue, $negate)
     {
         $this->constrain($filterState->getQuery(), $negate);
     }

extensions/subscriptions/src/Query/SubscriptionFilterGambit.php

@@ -11,6 +11,7 @@ namespace Flarum\Subscriptions\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\Search\AbstractRegexGambit;
 use Flarum\Search\SearchState;
 use Flarum\User\User;
@@ -18,6 +19,8 @@ use Illuminate\Database\Query\Builder;
 
 class SubscriptionFilterGambit extends AbstractRegexGambit implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     protected function getGambitPattern()
     {
         return 'is:(follow|ignor)(?:ing|ed)';
@@ -33,8 +36,10 @@ class SubscriptionFilterGambit extends AbstractRegexGambit implements FilterInte
         return 'subscription';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
+        $filterValue = $this->asString($filterValue);
+
         preg_match('/^'.$this->getGambitPattern().'$/i', 'is:'.$filterValue, $matches);
 
         $this->constrain($filterState->getQuery(), $filterState->getActor(), $matches[1], $negate);

extensions/suspend/src/Query/SuspendedFilterGambit.php

@@ -63,7 +63,7 @@ class SuspendedFilterGambit extends AbstractRegexGambit implements FilterInterfa
         return 'suspended';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         if (! $filterState->getActor()->can('suspend', new Guest())) {
             return false;

extensions/tags/src/Filter/PostTagFilter.php

@@ -11,18 +11,23 @@ namespace Flarum\Tags\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class PostTagFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'tag';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
+        $ids = $this->asIntArray($filterValue);
+
         $filterState->getQuery()
             ->join('discussion_tag', 'discussion_tag.discussion_id', '=', 'posts.discussion_id')
-            ->where('discussion_tag.tag_id', $negate ? '!=' : '=', $filterValue);
+            ->whereIn('discussion_tag.tag_id', $ids, 'and', $negate);
     }
 }

extensions/tags/src/Query/TagFilterGambit.php

@@ -11,6 +11,7 @@ namespace Flarum\Tags\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\Http\SlugManager;
 use Flarum\Search\AbstractRegexGambit;
 use Flarum\Search\SearchState;
@@ -21,6 +22,8 @@ use Illuminate\Database\Query\Builder;
 
 class TagFilterGambit extends AbstractRegexGambit implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * @var SlugManager
      */
@@ -46,14 +49,14 @@ class TagFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'tag';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         $this->constrain($filterState->getQuery(), $filterValue, $negate, $filterState->getActor());
     }
 
     protected function constrain(Builder $query, $rawSlugs, $negate, User $actor)
     {
-        $slugs = explode(',', trim($rawSlugs, '"'));
+        $slugs = $this->asStringArray($rawSlugs);
 
         $query->where(function (Builder $query) use ($slugs, $negate, $actor) {
             foreach ($slugs as $slug) {

framework/core/locale/core.yml

@@ -717,6 +717,10 @@ core:
   # Translations in this namespace are used in messages output by the API.
   api:
     invalid_username_message: "The username may only contain letters, numbers, and dashes."
+    invalid_filter_type:
+      must_be_numeric_message: "The {filter} filter must be numeric."
+      must_not_be_array_message: "The {filter} filter must not be an array."
+      must_not_be_multidimensional_array_message: "The {filter} filter must not be a multidimensional array."
 
   # Translations in this namespace are used in emails sent by the forum.
   email:

framework/core/src/Api/Controller/ListPostsController.php

@@ -145,7 +145,7 @@ class ListPostsController extends AbstractListController
                 );
             }
 
-            $offset = $this->posts->getIndexForNumber($filter['discussion'], $near, $actor);
+            $offset = $this->posts->getIndexForNumber((int) $filter['discussion'], $near, $actor);
 
             return max(0, $offset - $limit / 2);
         }

framework/core/src/Discussion/Query/AuthorFilterGambit.php

@@ -11,6 +11,7 @@ namespace Flarum\Discussion\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\Search\AbstractRegexGambit;
 use Flarum\Search\SearchState;
 use Flarum\User\UserRepository;
@@ -18,6 +19,8 @@ use Illuminate\Database\Query\Builder;
 
 class AuthorFilterGambit extends AbstractRegexGambit implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * @var \Flarum\User\UserRepository
      */
@@ -52,20 +55,16 @@ class AuthorFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'author';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         $this->constrain($filterState->getQuery(), $filterValue, $negate);
     }
 
     protected function constrain(Builder $query, $rawUsernames, $negate)
     {
-        $usernames = trim($rawUsernames, '"');
+        $usernames = $this->asStringArray($rawUsernames);
-        $usernames = explode(',', $usernames);
 
-        $ids = [];
+        $ids = $this->users->getIdsForUsernames($usernames);
-        foreach ($usernames as $username) {
-            $ids[] = $this->users->getIdForUsername($username);
-        }
 
         $query->whereIn('discussions.user_id', $ids, 'and', $negate);
     }

framework/core/src/Discussion/Query/CreatedFilterGambit.php

@@ -11,6 +11,7 @@ namespace Flarum\Discussion\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\Search\AbstractRegexGambit;
 use Flarum\Search\SearchState;
 use Illuminate\Database\Query\Builder;
@@ -18,6 +19,8 @@ use Illuminate\Support\Arr;
 
 class CreatedFilterGambit extends AbstractRegexGambit implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -39,8 +42,10 @@ class CreatedFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'created';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
+        $filterValue = $this->asString($filterValue);
+
         preg_match('/^'.$this->getGambitPattern().'$/i', 'created:'.$filterValue, $matches);
 
         $this->constrain($filterState->getQuery(), Arr::get($matches, 1), Arr::get($matches, 3), $negate);

framework/core/src/Discussion/Query/HiddenFilterGambit.php

@@ -38,7 +38,7 @@ class HiddenFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'hidden';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         $this->constrain($filterState->getQuery(), $negate);
     }

framework/core/src/Discussion/Query/UnreadFilterGambit.php

@@ -53,7 +53,7 @@ class UnreadFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'unread';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         $this->constrain($filterState->getQuery(), $filterState->getActor(), $negate);
     }

framework/core/src/Filter/FilterInterface.php

@@ -18,6 +18,8 @@ interface FilterInterface
 
     /**
      * Filters a query.
+     *
+     * @todo: 2.0 change the $filterValue type to mixed, as it can be an array.
      */
     public function filter(FilterState $filterState, string $filterValue, bool $negate);
 }

framework/core/src/Filter/ValidateFilterTrait.php

@@ -0,0 +1,94 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Filter;
+
+use Flarum\Foundation\ValidationException as FlarumValidationException;
+use Flarum\Locale\Translator;
+
+trait ValidateFilterTrait
+{
+    /**
+     * @throws FlarumValidationException
+     * @return array<string>|array<array>
+     */
+    protected function asStringArray($filterValue, bool $multidimensional = false): array
+    {
+        if (is_array($filterValue)) {
+            $value = array_map(function ($subValue) use ($multidimensional) {
+                if (is_array($subValue) && ! $multidimensional) {
+                    $this->throwValidationException('core.api.invalid_filter_type.must_not_be_multidimensional_array_message');
+                } elseif (is_array($subValue)) {
+                    return $this->asStringArray($subValue, true);
+                } else {
+                    return $this->asString($subValue);
+                }
+            }, $filterValue);
+        } else {
+            $value = explode(',', $this->asString($filterValue));
+        }
+
+        return $value;
+    }
+
+    /**
+     * @throws FlarumValidationException
+     */
+    protected function asString($filterValue): string
+    {
+        if (is_array($filterValue)) {
+            $this->throwValidationException('core.api.invalid_filter_type.must_not_be_array_message');
+        }
+
+        return trim($filterValue, '"');
+    }
+
+    /**
+     * @throws FlarumValidationException
+     */
+    protected function asInt($filterValue): int
+    {
+        if (! is_numeric($filterValue)) {
+            $this->throwValidationException('core.api.invalid_filter_type.must_be_numeric_message');
+        }
+
+        return (int) $this->asString($filterValue);
+    }
+
+    /**
+     * @throws FlarumValidationException
+     * @return array<int>
+     */
+    protected function asIntArray($filterValue): array
+    {
+        return array_map(function ($value) {
+            return $this->asInt($value);
+        }, $this->asStringArray($filterValue));
+    }
+
+    /**
+     * @throws FlarumValidationException
+     */
+    protected function asBool($filterValue): bool
+    {
+        return $this->asString($filterValue) === '1';
+    }
+
+    /**
+     * @throws FlarumValidationException
+     */
+    private function throwValidationException(string $messageCode): void
+    {
+        $translator = resolve(Translator::class);
+
+        throw new FlarumValidationException([
+            'message' => $translator->trans($messageCode, ['{filter}' => $this->getFilterKey()]),
+        ]);
+    }
+}

framework/core/src/Group/Filter/HiddenFilter.php

@@ -11,16 +11,21 @@ namespace Flarum\Group\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class HiddenFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'hidden';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $filterState->getQuery()->where('is_hidden', $negate ? '!=' : '=', $filterValue);
+        $hidden = $this->asBool($filterValue);
+
+        $filterState->getQuery()->where('is_hidden', $negate ? '!=' : '=', $hidden);
     }
 }

framework/core/src/Http/Filter/UserFilter.php

@@ -12,6 +12,7 @@ namespace Flarum\Http\Filter;
 use Flarum\Api\Controller\ListAccessTokensController;
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 /**
  * Filters an access tokens request by the related user.
@@ -20,6 +21,8 @@ use Flarum\Filter\FilterState;
  */
 class UserFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * @inheritDoc
      */
@@ -31,8 +34,10 @@ class UserFilter implements FilterInterface
     /**
      * @inheritDoc
      */
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
+        $filterValue = $this->asInt($filterValue);
+
         $filterState->getQuery()->where('user_id', $negate ? '!=' : '=', $filterValue);
     }
 }

framework/core/src/Post/Filter/AuthorFilter.php

@@ -11,18 +11,18 @@ namespace Flarum\Post\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\User\UserRepository;
 
 class AuthorFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * @var \Flarum\User\UserRepository
      */
     protected $users;
 
-    /**
-     * @param \Flarum\User\UserRepository $users
-     */
     public function __construct(UserRepository $users)
     {
         $this->users = $users;
@@ -33,10 +33,9 @@ class AuthorFilter implements FilterInterface
         return 'author';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $usernames = trim($filterValue, '"');
+        $usernames = $this->asStringArray($filterValue);
-        $usernames = explode(',', $usernames);
 
         $ids = $this->users->query()->whereIn('username', $usernames)->pluck('id');
 

framework/core/src/Post/Filter/DiscussionFilter.php

@@ -11,17 +11,20 @@ namespace Flarum\Post\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class DiscussionFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'discussion';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $discussionId = trim($filterValue, '"');
+        $discussionId = $this->asInt($filterValue);
 
         $filterState->getQuery()->where('posts.discussion_id', $negate ? '!=' : '=', $discussionId);
     }

framework/core/src/Post/Filter/IdFilter.php

@@ -11,18 +11,20 @@ namespace Flarum\Post\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class IdFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'id';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $idString = trim($filterValue, '"');
+        $ids = $this->asIntArray($filterValue);
-        $ids = explode(',', $idString);
 
         $filterState->getQuery()->whereIn('posts.id', $ids, 'and', $negate);
     }

framework/core/src/Post/Filter/NumberFilter.php

@@ -11,17 +11,20 @@ namespace Flarum\Post\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class NumberFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'number';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $number = trim($filterValue, '"');
+        $number = $this->asInt($filterValue);
 
         $filterState->getQuery()->where('posts.number', $negate ? '!=' : '=', $number);
     }

framework/core/src/Post/Filter/TypeFilter.php

@@ -11,17 +11,20 @@ namespace Flarum\Post\Filter;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 
 class TypeFilter implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     public function getFilterKey(): string
     {
         return 'type';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
-        $type = trim($filterValue, '"');
+        $type = $this->asString($filterValue);
 
         $filterState->getQuery()->where('posts.type', $negate ? '!=' : '=', $type);
     }

framework/core/src/Post/PostRepository.php

@@ -105,8 +105,11 @@ class PostRepository
      */
     public function getIndexForNumber($discussionId, $number, User $actor = null)
     {
-        $query = Discussion::find($discussionId)
+        if (! ($discussion = Discussion::find($discussionId))) {
-            ->posts()
+            return 0;
+        }
+
+        $query = $discussion->posts()
             ->whereVisibleTo($actor)
             ->where('created_at', '<', function ($query) use ($discussionId, $number) {
                 $query->select('created_at')

framework/core/src/User/Query/EmailFilterGambit.php

@@ -11,12 +11,15 @@ namespace Flarum\User\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\Search\AbstractRegexGambit;
 use Flarum\Search\SearchState;
 use Illuminate\Database\Query\Builder;
 
 class EmailFilterGambit extends AbstractRegexGambit implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -50,7 +53,7 @@ class EmailFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'email';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         if (! $filterState->getActor()->hasPermission('user.edit')) {
             return;
@@ -61,7 +64,7 @@ class EmailFilterGambit extends AbstractRegexGambit implements FilterInterface
 
     protected function constrain(Builder $query, $rawEmail, bool $negate)
     {
-        $email = trim($rawEmail, '"');
+        $email = $this->asString($rawEmail);
 
         $query->where('email', $negate ? '!=' : '=', $email);
     }

framework/core/src/User/Query/GroupFilterGambit.php

@@ -11,6 +11,7 @@ namespace Flarum\User\Query;
 
 use Flarum\Filter\FilterInterface;
 use Flarum\Filter\FilterState;
+use Flarum\Filter\ValidateFilterTrait;
 use Flarum\Group\Group;
 use Flarum\Search\AbstractRegexGambit;
 use Flarum\Search\SearchState;
@@ -19,6 +20,8 @@ use Illuminate\Database\Query\Builder;
 
 class GroupFilterGambit extends AbstractRegexGambit implements FilterInterface
 {
+    use ValidateFilterTrait;
+
     /**
      * {@inheritdoc}
      */
@@ -40,15 +43,14 @@ class GroupFilterGambit extends AbstractRegexGambit implements FilterInterface
         return 'group';
     }
 
-    public function filter(FilterState $filterState, string $filterValue, bool $negate)
+    public function filter(FilterState $filterState, $filterValue, bool $negate)
     {
         $this->constrain($filterState->getQuery(), $filterState->getActor(), $filterValue, $negate);
     }
 
-    protected function constrain(Builder $query, User $actor, string $rawQuery, bool $negate)
+    protected function constrain(Builder $query, User $actor, $rawQuery, bool $negate)
     {
-        $groupIdentifiers = explode(',', trim($rawQuery, '"'));
+        $groupIdentifiers = $this->asStringArray($rawQuery);
-
         $groupQuery = Group::whereVisibleTo($actor);
 
         $ids = [];

framework/core/src/User/UserRepository.php

@@ -95,6 +95,13 @@ class UserRepository
         return $this->scopeVisibleTo($query, $actor)->value('id');
     }
 
+    public function getIdsForUsernames(array $usernames, User $actor = null): array
+    {
+        $query = $this->query()->whereIn('username', $usernames);
+
+        return $this->scopeVisibleTo($query, $actor)->pluck('id')->all();
+    }
+
     /**
      * Find users by matching a string of words against their username,
      * optionally making sure they are visible to a certain user.