CVE-2021-41082 (Ruby, published 2021-09-20T21:15Z)
Description: value: Discourse is a platform for community discussion. In affected versions any private message that includes a group had its title and participating user exposed to users that do not have access to the private messages. However, access control for the private messages was not compromised as users were not able to view the posts in the leaked private message despite seeing it in their inbox. The problematic commit was reverted around 32 minutes after it was made. Users are encouraged to upgrade to the latest commit if they are running Discourse against the `tests-passed` branch.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-863 \u2014 Incorrect Authorization
The product performs an authorization check when an actor attempts to access a resource or perform an action, but it does not correctly perform the check. This allows attackers to bypass intended access restrictions.
Repository: https://github.com/discourse/discourse
Files changed:
app/controllers/topics_controller.rb
lib/topic_query/private_message_lists.rb
spec/lib/topic_query/private_message_lists_spec.rb
Commit message: PERF: Improve query performance all inbox private messages. (#14304)

First reported in https://meta.discourse.org/t/-/202482/19

There are two optimizations being applied here:

1. Fetch a user's group ids in a seperate query instead of including it
   as a sub-query. When I tried a subquery, the query plan becomes very
inefficient.

1. Join against the `topic_allowed_users` and `topic_allowed_groups`
   table instead of doing an IN against a subquery where we UNION the
`topic_id`s from the two tables. From my profiling, this enables PG to
do a backwards index scan on the `index_topics_on_timestamps_private`
index.

This commit fixes a bug where listing all messages was incorrectly
excluding topics if a topic has been archived by a group even if the
user did not belong to the group.

This commit also fixes another bug where dismissing private messages
selectively was subjected to the default limit of 30.

Vulnerable code:
    topic_query = TopicQuery.new(current_user)
      topic_query.options[:limit] = false
      topic_scope.pluck(:id),
      topics = topic_query.filter_private_messages_unread(current_user, filter)
    end
    topics.pluck(:id)
        result = result.where("topics.id IN (
              SELECT topic_id
              FROM topic_allowed_users
              WHERE user_id = #{user.id.to_i}
              UNION ALL
              SELECT topic_id FROM topic_allowed_groups
              WHERE group_id IN (
                SELECT group_id FROM group_users WHERE user_id = #{user.id.to_i}
              )
      )")
      LEFT JOIN group_archived_messages gm ON gm.topic_id = topics.id
      GroupArchivedMessage.archive!(user_2.id, group_message)
      GroupArchivedMessage.archive!(user_2.id, group_message)
      GroupArchivedMessage.archive!(user_2.id, group_message)

Fixed code:
    topic_query = TopicQuery.new(current_user, limit: false)
      topic_scope.distinct(false).pluck(:id),

      topic_query
        .filter_private_messages_unread(current_user, filter)
        .distinct(false)
        .pluck(:id)
      topics.pluck(:id)
    end
        group_ids = group_with_messages_ids(user)

        result = result.joins(<<~SQL)
        LEFT JOIN topic_allowed_users tau
          ON tau.topic_id = topics.id
          AND tau.user_id = #{user.id.to_i}
        LEFT JOIN topic_allowed_groups tag
          ON tag.topic_id = topics.id
          #{group_ids.present? ? "AND tag.group_id IN (#{group_ids.join(",")})" : ""}
        SQL

        result = result
          .where("tag.topic_id IS NOT NULL OR tau.topic_id IS NOT NULL")
          .distinct
      # Executing an extra query instead of a sub-query because it is more
      # efficient for the PG planner. Caution should be used when changing the
      # query here as it can easily lead to an inefficient query.
      group_ids = group_with_messages_ids(user)

      LEFT JOIN group_archived_messages gm
        ON gm.topic_id = topics.id
        #{group_ids.present? ? "AND gm.group_id IN (#{group_ids.join(",")})" : ""}

    def group_with_messages_ids(user)
      @group_with_messages_ids ||= {}

      if ids = @group_with_messages_ids[user.id]
        return ids
      end

      @group_with_messages_ids[user.id] = user.groups.where(has_messages: true).pluck(:id)
    end
      GroupArchivedMessage.archive!(group.id, group_message)
      GroupArchivedMessage.archive!(group.id, group_message)
      GroupArchivedMessage.archive!(group.id, group_message)