In `XF\Pub\Controller\Search`, `actionResults` is missing checks from `actionSearch`

[Xon]

Affected version

2.2.11

In practice, The XF\Pub\Controller\Search::actionResults endpoint is laregly a replica of the XF\Pub\Controller\Search::actionSearch method, but with fewer checks.

• \XF::visitor()->canSearch() is not checked.
• When re-running search, it does not call $searcher->isQueryEmpty() and instead half bakes this depending on if it is a user vs a guest.
• Search is always re-run for logged in users even for an empty query or if they are not allowed todo searches.
• The changed search check for guests ( $search->search_query !== $this->filter('q', 'str')) is not sufficient to detect if this is a different search.

This set of changes means a logged in user can always craft search queries.

 

[Xon]

There are a number of endpoints on the Search controller which are missing canSearch checks, but look like they should.

Maybe just put that in the preDispatch function so every action has that check done by default?

 

[Xon]

actionResult returns a not-found instead of an error message if parsing the query fails, unlike actionSearch which gives an actual error message:

            if ($query->getErrors())
            {
                return $this->notFound();
            }

 

[Xon]

Basically this accumulates in a link such as https://xenforo.com/community/search/900000000/ doing a full search of all content without returning some sort of error for users (ie an invalid search id, either from expiring or never existing in the first place).

 

[Xon]

Another bug;

Taking a URL from a guest session (without any arguments, ie from a member search results) and using it in a user's session results a search without any filters. ie a search of everything.

 

[Xon]

If $search != null, but is owned by a member and accessed by a guest, for a search without a search_query (ie keywords) it falls through.
However, search results are checked for if the guest can view them. But it does hint based if a term exists privately by repeating the same search as a guest.

The workaround I'm using:

    public function actionResults(ParameterBag $params)
    {
        $visitor = \XF::visitor();

        // existence + ownership checks
        $visitorId = (int)$visitor->user_id;
        /** @var \XF\Entity\Search|null $search */
        $search = $this->em()->find('XF:Search', (int)$params->get('search_id'));
        if ($search === null || $search->user_id !== $visitorId) {
            if ($visitorId === 0) {
                // prevent sharing of search links from members to guests
                return $this->message(\XF::phrase('no_results_found'));
            }

            $searchData = $this->convertShortSearchInputNames();
            $query = $this->prepareSearchQuery($searchData, $constraints);
            if ($query->getErrors()) {
                return $this->error($query->getErrors());
            }
            $searcher = $this->app->search();
            if ($searcher->isQueryEmpty($query, $error)) {
                return $this->error($error);
            }

            // always re-run search for logged-in users
            return $this->runSearch($query, $constraints);
        }
        elseif ($visitorId === 0) {
            // Determine if the search query (broadly) matches the stored query
            $searchData = $this->convertShortSearchInputNames();
            $storedArgs = $this->convertSearchToQueryInput($search);
            // Use non-exact compare as it is recursively insensitive to element order
            if ($searchData != $storedArgs) {
                return $this->message(\XF::phrase('no_results_found'));
            }
        }

        return parent::actionResults($params);
    }

It assumes the patch in this thread has been applied so the query URL always should contain the search query:

Fixed - All redirects to search results should include search query arguments

The current ownership checks assume the search query arguments are always included so the query can be re-run. But this isn't the case and causes various bugs. Do a member search in a guest session and copy & paste that into a member session, get complete nonsense as search results.

xenforo.com