Merge tag 'v3.4.6' of https://github.com/tootsuite/mastodon

Fix insufficient sanitization of report comments (#17430 )
Bump version to 3.4.6
2022-02-03 06:47:40 -07:00 · 2022-02-03 14:08:24 +01:00 · 2022-02-02 23:48:38 +01:00 · 2022-02-02 23:30:15 +01:00 · 2022-02-02 23:30:15 +01:00 · 2022-02-02 23:30:15 +01:00
18 changed files with 408 additions and 204 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -3,6 +3,18 @@ Changelog

 All notable changes to this project will be documented in this file.

+## [3.4.6] - 2022-02-03
+### Fixed
+- Fix `mastodon:webpush:generate_vapid_key` task requiring a functional environment ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/17338))
+- Fix spurious errors when receiving an Add activity for a private post ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/17425))
+
+### Security
+- Fix error-prone SQL queries ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/15828))
+- Fix not compacting incoming signed JSON-LD activities ([puckipedia](https://github.com/mastodon/mastodon/pull/17426), [ClearlyClaire](https://github.com/mastodon/mastodon/pull/17428)) (CVE-2022-24307)
+- Fix insufficient sanitization of report comments ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/17430))
+- Fix stop condition of a Common Table Expression ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/17427))
+- Disable legacy XSS filtering ([Wonderfall](https://github.com/mastodon/mastodon/pull/17289))
+
 ## [3.4.5] - 2022-01-31
 ### Added
 - Add more advanced migration tests ([ClearlyClaire](https://github.com/mastodon/mastodon/pull/17393))
--- a/app/helpers/context_helper.rb
+++ b/app/helpers/context_helper.rb
@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+module ContextHelper
+  NAMED_CONTEXT_MAP = {
+    activitystreams: 'https://www.w3.org/ns/activitystreams',
+    security: 'https://w3id.org/security/v1',
+  }.freeze
+
+  CONTEXT_EXTENSION_MAP = {
+    manually_approves_followers: { 'manuallyApprovesFollowers' => 'as:manuallyApprovesFollowers' },
+    sensitive: { 'sensitive' => 'as:sensitive' },
+    hashtag: { 'Hashtag' => 'as:Hashtag' },
+    moved_to: { 'movedTo' => { '@id' => 'as:movedTo', '@type' => '@id' } },
+    also_known_as: { 'alsoKnownAs' => { '@id' => 'as:alsoKnownAs', '@type' => '@id' } },
+    emoji: { 'toot' => 'http://joinmastodon.org/ns#', 'Emoji' => 'toot:Emoji' },
+    featured: { 'toot' => 'http://joinmastodon.org/ns#', 'featured' => { '@id' => 'toot:featured', '@type' => '@id' }, 'featuredTags' => { '@id' => 'toot:featuredTags', '@type' => '@id' } },
+    property_value: { 'schema' => 'http://schema.org#', 'PropertyValue' => 'schema:PropertyValue', 'value' => 'schema:value' },
+    atom_uri: { 'ostatus' => 'http://ostatus.org#', 'atomUri' => 'ostatus:atomUri' },
+    conversation: { 'ostatus' => 'http://ostatus.org#', 'inReplyToAtomUri' => 'ostatus:inReplyToAtomUri', 'conversation' => 'ostatus:conversation' },
+    focal_point: { 'toot' => 'http://joinmastodon.org/ns#', 'focalPoint' => { '@container' => '@list', '@id' => 'toot:focalPoint' } },
+    identity_proof: { 'toot' => 'http://joinmastodon.org/ns#', 'IdentityProof' => 'toot:IdentityProof' },
+    blurhash: { 'toot' => 'http://joinmastodon.org/ns#', 'blurhash' => 'toot:blurhash' },
+    discoverable: { 'toot' => 'http://joinmastodon.org/ns#', 'discoverable' => 'toot:discoverable' },
+    voters_count: { 'toot' => 'http://joinmastodon.org/ns#', 'votersCount' => 'toot:votersCount' },
+    olm: { 'toot' => 'http://joinmastodon.org/ns#', 'Device' => 'toot:Device', 'Ed25519Signature' => 'toot:Ed25519Signature', 'Ed25519Key' => 'toot:Ed25519Key', 'Curve25519Key' => 'toot:Curve25519Key', 'EncryptedMessage' => 'toot:EncryptedMessage', 'publicKeyBase64' => 'toot:publicKeyBase64', 'deviceId' => 'toot:deviceId', 'claim' => { '@type' => '@id', '@id' => 'toot:claim' }, 'fingerprintKey' => { '@type' => '@id', '@id' => 'toot:fingerprintKey' }, 'identityKey' => { '@type' => '@id', '@id' => 'toot:identityKey' }, 'devices' => { '@type' => '@id', '@id' => 'toot:devices' }, 'messageFranking' => 'toot:messageFranking', 'messageType' => 'toot:messageType', 'cipherText' => 'toot:cipherText' },
+    suspended: { 'toot' => 'http://joinmastodon.org/ns#', 'suspended' => 'toot:suspended' },
+  }.freeze
+
+  def full_context
+    serialized_context(NAMED_CONTEXT_MAP, CONTEXT_EXTENSION_MAP)
+  end
+
+  def serialized_context(named_contexts_map, context_extensions_map)
+    context_array = []
+
+    named_contexts     = named_contexts_map.keys
+    context_extensions = context_extensions_map.keys
+
+    named_contexts.each do |key|
+      context_array << NAMED_CONTEXT_MAP[key]
+    end
+
+    extensions = context_extensions.each_with_object({}) do |key, h|
+      h.merge!(CONTEXT_EXTENSION_MAP[key])
+    end
+
+    context_array << extensions unless extensions.empty?
+
+    if context_array.size == 1
+      context_array.first
+    else
+      context_array
+    end
+  end
+end
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@ -1,6 +1,8 @@
 # frozen_string_literal: true

 module JsonLdHelper
+  include ContextHelper
+
  def equals_or_includes?(haystack, needle)
    haystack.is_a?(Array) ? haystack.include?(needle) : haystack == needle
  end
@ -63,6 +65,84 @@ module JsonLdHelper
    graph.dump(:normalize)
  end

+  def compact(json)
+    compacted = JSON::LD::API.compact(json.without('signature'), full_context, documentLoader: method(:load_jsonld_context))
+    compacted['signature'] = json['signature']
+    compacted
+  end
+
+  # Patches a JSON-LD document to avoid compatibility issues on redistribution
+  #
+  # Since compacting a JSON-LD document against Mastodon's built-in vocabulary
+  # means other extension namespaces will be expanded, malformed JSON-LD
+  # attributes lost, and some values “unexpectedly” compacted this method
+  # patches the following likely sources of incompatibility:
+  # - 'https://www.w3.org/ns/activitystreams#Public' being compacted to
+  #   'as:Public' (for instance, pre-3.4.0 Mastodon does not understand
+  #   'as:Public')
+  # - single-item arrays being compacted to the item itself (`[foo]` being
+  #   compacted to `foo`)
+  #
+  # It is not always possible for `patch_for_forwarding!` to produce a document
+  # deemed safe for forwarding. Use `safe_for_forwarding?` to check the status
+  # of the output document.
+  #
+  # @param original [Hash] The original JSON-LD document used as reference
+  # @param compacted [Hash] The compacted JSON-LD document to be patched
+  # @return [void]
+  def patch_for_forwarding!(original, compacted)
+    original.without('@context', 'signature').each do |key, value|
+      next if value.nil? || !compacted.key?(key)
+
+      compacted_value = compacted[key]
+      if value.is_a?(Hash) && compacted_value.is_a?(Hash)
+        patch_for_forwarding!(value, compacted_value)
+      elsif value.is_a?(Array)
+        compacted_value = [compacted_value] unless compacted_value.is_a?(Array)
+        return if value.size != compacted_value.size
+
+        compacted[key] = value.zip(compacted_value).map do |v, vc|
+          if v.is_a?(Hash) && vc.is_a?(Hash)
+            patch_for_forwarding!(v, vc)
+            vc
+          elsif v == 'https://www.w3.org/ns/activitystreams#Public' && vc == 'as:Public'
+            v
+          else
+            vc
+          end
+        end
+      elsif value == 'https://www.w3.org/ns/activitystreams#Public' && compacted_value == 'as:Public'
+        compacted[key] = value
+      end
+    end
+  end
+
+  # Tests whether a JSON-LD compaction is deemed safe for redistribution,
+  # that is, if it doesn't change its meaning to consumers that do not actually
+  # handle JSON-LD, but rely on values being serialized in a certain way.
+  #
+  # See `patch_for_forwarding!` for details.
+  #
+  # @param original [Hash] The original JSON-LD document used as reference
+  # @param compacted [Hash] The compacted JSON-LD document to be patched
+  # @return [Boolean] Whether the patched document is deemed safe
+  def safe_for_forwarding?(original, compacted)
+    original.without('@context', 'signature').all? do |key, value|
+      compacted_value = compacted[key]
+      return false unless value.class == compacted_value.class
+
+      if value.is_a?(Hash)
+        safe_for_forwarding?(value, compacted_value)
+      elsif value.is_a?(Array)
+        value.zip(compacted_value).all? do |v, vc|
+          v.is_a?(Hash) ? (vc.is_a?(Hash) && safe_for_forwarding?(v, vc)) : v == vc
+        end
+      else
+        value == compacted_value
+      end
+    end
+  end
+
  def fetch_resource(uri, id, on_behalf_of = nil)
    unless id
      json = fetch_resource_without_id_validation(uri, on_behalf_of)
--- a/app/lib/activitypub/activity/add.rb
+++ b/app/lib/activitypub/activity/add.rb
@ -7,7 +7,7 @@ class ActivityPub::Activity::Add < ActivityPub::Activity
    status   = status_from_uri(object_uri)
    status ||= fetch_remote_original_status

-    return unless !status.nil? && status.account_id == @account.id && !@account.pinned?(status)
+    return unless !status.nil? && status.account_id == @account.id && !@account.pinned?(status) && status.distributable?

    StatusPin.create!(account: @account, status: status)
  end
--- a/app/lib/activitypub/adapter.rb
+++ b/app/lib/activitypub/adapter.rb
@ -1,30 +1,7 @@
 # frozen_string_literal: true

 class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base
-  NAMED_CONTEXT_MAP = {
-    activitystreams: 'https://www.w3.org/ns/activitystreams',
-    security: 'https://w3id.org/security/v1',
-  }.freeze
-
-  CONTEXT_EXTENSION_MAP = {
-    manually_approves_followers: { 'manuallyApprovesFollowers' => 'as:manuallyApprovesFollowers' },
-    sensitive: { 'sensitive' => 'as:sensitive' },
-    hashtag: { 'Hashtag' => 'as:Hashtag' },
-    moved_to: { 'movedTo' => { '@id' => 'as:movedTo', '@type' => '@id' } },
-    also_known_as: { 'alsoKnownAs' => { '@id' => 'as:alsoKnownAs', '@type' => '@id' } },
-    emoji: { 'toot' => 'http://joinmastodon.org/ns#', 'Emoji' => 'toot:Emoji' },
-    featured: { 'toot' => 'http://joinmastodon.org/ns#', 'featured' => { '@id' => 'toot:featured', '@type' => '@id' }, 'featuredTags' => { '@id' => 'toot:featuredTags', '@type' => '@id' } },
-    property_value: { 'schema' => 'http://schema.org#', 'PropertyValue' => 'schema:PropertyValue', 'value' => 'schema:value' },
-    atom_uri: { 'ostatus' => 'http://ostatus.org#', 'atomUri' => 'ostatus:atomUri' },
-    conversation: { 'ostatus' => 'http://ostatus.org#', 'inReplyToAtomUri' => 'ostatus:inReplyToAtomUri', 'conversation' => 'ostatus:conversation' },
-    focal_point: { 'toot' => 'http://joinmastodon.org/ns#', 'focalPoint' => { '@container' => '@list', '@id' => 'toot:focalPoint' } },
-    identity_proof: { 'toot' => 'http://joinmastodon.org/ns#', 'IdentityProof' => 'toot:IdentityProof' },
-    blurhash: { 'toot' => 'http://joinmastodon.org/ns#', 'blurhash' => 'toot:blurhash' },
-    discoverable: { 'toot' => 'http://joinmastodon.org/ns#', 'discoverable' => 'toot:discoverable' },
-    voters_count: { 'toot' => 'http://joinmastodon.org/ns#', 'votersCount' => 'toot:votersCount' },
-    olm: { 'toot' => 'http://joinmastodon.org/ns#', 'Device' => 'toot:Device', 'Ed25519Signature' => 'toot:Ed25519Signature', 'Ed25519Key' => 'toot:Ed25519Key', 'Curve25519Key' => 'toot:Curve25519Key', 'EncryptedMessage' => 'toot:EncryptedMessage', 'publicKeyBase64' => 'toot:publicKeyBase64', 'deviceId' => 'toot:deviceId', 'claim' => { '@type' => '@id', '@id' => 'toot:claim' }, 'fingerprintKey' => { '@type' => '@id', '@id' => 'toot:fingerprintKey' }, 'identityKey' => { '@type' => '@id', '@id' => 'toot:identityKey' }, 'devices' => { '@type' => '@id', '@id' => 'toot:devices' }, 'messageFranking' => 'toot:messageFranking', 'messageType' => 'toot:messageType', 'cipherText' => 'toot:cipherText' },
-    suspended: { 'toot' => 'http://joinmastodon.org/ns#', 'suspended' => 'toot:suspended' },
-  }.freeze
+  include ContextHelper

  def self.default_key_transform
    :camel_lower
@ -35,7 +12,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base
  end

  def serializable_hash(options = nil)
-    named_contexts     = {}
+    named_contexts     = { activitystreams: NAMED_CONTEXT_MAP['activitystreams'] }
    context_extensions = {}

    options         = serialization_options(options)
@ -45,29 +22,4 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base

    { '@context' => serialized_context(named_contexts, context_extensions) }.merge(serialized_hash)
  end
-
-  private
-
-  def serialized_context(named_contexts_map, context_extensions_map)
-    context_array = []
-
-    named_contexts     = [:activitystreams] + named_contexts_map.keys
-    context_extensions = context_extensions_map.keys
-
-    named_contexts.each do |key|
-      context_array << NAMED_CONTEXT_MAP[key]
-    end
-
-    extensions = context_extensions.each_with_object({}) do |key, h|
-      h.merge!(CONTEXT_EXTENSION_MAP[key])
-    end
-
-    context_array << extensions unless extensions.empty?
-
-    if context_array.size == 1
-      context_array.first
-    else
-      context_array
-    end
-  end
 end
--- a/app/models/account.rb
+++ b/app/models/account.rb
@ -426,6 +426,9 @@ class Account < ApplicationRecord
  end

  class << self
+    DISALLOWED_TSQUERY_CHARACTERS = /['?\\:‘’]/.freeze
+    TEXTSEARCH = "(setweight(to_tsvector('simple', accounts.display_name), 'A') || setweight(to_tsvector('simple', accounts.username), 'B') || setweight(to_tsvector('simple', coalesce(accounts.domain, '')), 'C'))"
+
    def readonly_attributes
      super - %w(statuses_count following_count followers_count)
    end
@ -436,70 +439,29 @@ class Account < ApplicationRecord
    end

    def search_for(terms, limit = 10, offset = 0)
-      textsearch, query = generate_query_for_search(terms)
+      tsquery = generate_query_for_search(terms)

      sql = <<-SQL.squish
        SELECT
          accounts.*,
-          ts_rank_cd(#{textsearch}, #{query}, 32) AS rank
+          ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank
        FROM accounts
-        WHERE #{query} @@ #{textsearch}
+        WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH}
          AND accounts.suspended_at IS NULL
          AND accounts.moved_to_account_id IS NULL
        ORDER BY rank DESC
-        LIMIT ? OFFSET ?
+        LIMIT :limit OFFSET :offset
      SQL

-      records = find_by_sql([sql, limit, offset])
+      records = find_by_sql([sql, limit: limit, offset: offset, tsquery: tsquery])
      ActiveRecord::Associations::Preloader.new.preload(records, :account_stat)
      records
    end

    def advanced_search_for(terms, account, limit = 10, following = false, offset = 0)
-      textsearch, query = generate_query_for_search(terms)
-
-      if following
-        sql = <<-SQL.squish
-          WITH first_degree AS (
-            SELECT target_account_id
-            FROM follows
-            WHERE account_id = ?
-            UNION ALL
-            SELECT ?
-          )
-          SELECT
-            accounts.*,
-            (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank
-          FROM accounts
-          LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?)
-          WHERE accounts.id IN (SELECT * FROM first_degree)
-            AND #{query} @@ #{textsearch}
-            AND accounts.suspended_at IS NULL
-            AND accounts.moved_to_account_id IS NULL
-          GROUP BY accounts.id
-          ORDER BY rank DESC
-          LIMIT ? OFFSET ?
-        SQL
-
-        records = find_by_sql([sql, account.id, account.id, account.id, limit, offset])
-      else
-        sql = <<-SQL.squish
-          SELECT
-            accounts.*,
-            (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank
-          FROM accounts
-          LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?)
-          WHERE #{query} @@ #{textsearch}
-            AND accounts.suspended_at IS NULL
-            AND accounts.moved_to_account_id IS NULL
-          GROUP BY accounts.id
-          ORDER BY rank DESC
-          LIMIT ? OFFSET ?
-        SQL
-
-        records = find_by_sql([sql, account.id, account.id, limit, offset])
-      end
-
+      tsquery = generate_query_for_search(terms)
+      sql = advanced_search_for_sql_template(following)
+      records = find_by_sql([sql, id: account.id, limit: limit, offset: offset, tsquery: tsquery])
      ActiveRecord::Associations::Preloader.new.preload(records, :account_stat)
      records
    end
@ -521,12 +483,55 @@ class Account < ApplicationRecord

    private

-    def generate_query_for_search(terms)
-      terms      = Arel.sql(connection.quote(terms.gsub(/['?\\:]/, ' ')))
-      textsearch = "(setweight(to_tsvector('simple', accounts.display_name), 'A') || setweight(to_tsvector('simple', accounts.username), 'B') || setweight(to_tsvector('simple', coalesce(accounts.domain, '')), 'C'))"
-      query      = "to_tsquery('simple', ''' ' || #{terms} || ' ''' || ':*')"
+    def generate_query_for_search(unsanitized_terms)
+      terms = unsanitized_terms.gsub(DISALLOWED_TSQUERY_CHARACTERS, ' ')

-      [textsearch, query]
+      # The final ":*" is for prefix search.
+      # The trailing space does not seem to fit any purpose, but `to_tsquery`
+      # behaves differently with and without a leading space if the terms start
+      # with `./`, `../`, or `.. `. I don't understand why, so, in doubt, keep
+      # the same query.
+      "' #{terms} ':*"
+    end
+
+    def advanced_search_for_sql_template(following)
+      if following
+        <<-SQL.squish
+          WITH first_degree AS (
+            SELECT target_account_id
+            FROM follows
+            WHERE account_id = :id
+            UNION ALL
+            SELECT :id
+          )
+          SELECT
+            accounts.*,
+            (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank
+          FROM accounts
+          LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id)
+          WHERE accounts.id IN (SELECT * FROM first_degree)
+            AND to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH}
+            AND accounts.suspended_at IS NULL
+            AND accounts.moved_to_account_id IS NULL
+          GROUP BY accounts.id
+          ORDER BY rank DESC
+          LIMIT :limit OFFSET :offset
+        SQL
+      else
+        <<-SQL.squish
+          SELECT
+            accounts.*,
+            (count(f.id) + 1) * ts_rank_cd(#{TEXTSEARCH}, to_tsquery('simple', :tsquery), 32) AS rank
+          FROM accounts
+          LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = :id) OR (accounts.id = f.target_account_id AND f.account_id = :id)
+          WHERE to_tsquery('simple', :tsquery) @@ #{TEXTSEARCH}
+            AND accounts.suspended_at IS NULL
+            AND accounts.moved_to_account_id IS NULL
+          GROUP BY accounts.id
+          ORDER BY rank DESC
+          LIMIT :limit OFFSET :offset
+        SQL
+      end
    end
  end

--- a/app/models/status.rb
+++ b/app/models/status.rb
@ -96,15 +96,12 @@ class Status < ApplicationRecord
  scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) }
  scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) }
  scope :tagged_with_all, ->(tag_ids) {
-    Array(tag_ids).reduce(self) do |result, id|
+    Array(tag_ids).map(&:to_i).reduce(self) do |result, id|
      result.joins("INNER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}")
    end
  }
  scope :tagged_with_none, ->(tag_ids) {
-    Array(tag_ids).reduce(self) do |result, id|
-      result.joins("LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}")
-            .where("t#{id}.tag_id IS NULL")
-    end
+    where('NOT EXISTS (SELECT * FROM statuses_tags forbidden WHERE forbidden.status_id = statuses.id AND forbidden.tag_id IN (?))', tag_ids)
  }

  cache_associated :application,
--- a/app/services/activitypub/process_collection_service.rb
+++ b/app/services/activitypub/process_collection_service.rb
@ -5,11 +5,27 @@ class ActivityPub::ProcessCollectionService < BaseService

  def call(body, account, **options)
    @account = account
-    @json    = Oj.load(body, mode: :strict)
+    @json    = original_json = Oj.load(body, mode: :strict)
    @options = options

+    begin
+      @json = compact(@json) if @json['signature'].is_a?(Hash)
+    rescue JSON::LD::JsonLdError => e
+      Rails.logger.debug "Error when compacting JSON-LD document for #{value_or_id(@json['actor'])}: #{e.message}"
+      @json = original_json.without('signature')
+    end
+
    return if !supported_context? || (different_actor? && verify_account!.nil?) || suspended_actor? || @account.local?

+    if @json['signature'].present?
+      # We have verified the signature, but in the compaction step above, might
+      # have introduced incompatibilities with other servers that do not
+      # normalize the JSON-LD documents (for instance, previous Mastodon
+      # versions), so skip redistribution if we can't get a safe document.
+      patch_for_forwarding!(original_json, @json)
+      @json.delete('signature') unless safe_for_forwarding?(original_json, @json)
+    end
+
    case @json['type']
    when 'Collection', 'CollectionPage'
      process_items @json['items']
--- a/app/services/notify_service.rb
+++ b/app/services/notify_service.rb
@ -73,9 +73,11 @@ class NotifyService < BaseService

    # Using an SQL CTE to avoid unneeded back-and-forth with SQL server in case of long threads
    !Status.count_by_sql([<<-SQL.squish, id: @notification.target_status.in_reply_to_id, recipient_id: @recipient.id, sender_id: @notification.from_account.id]).zero?
-      WITH RECURSIVE ancestors(id, in_reply_to_id, replying_to_sender) AS (
+      WITH RECURSIVE ancestors(id, in_reply_to_id, replying_to_sender, path) AS (
          SELECT
-            s.id, s.in_reply_to_id, (CASE
+            s.id,
+            s.in_reply_to_id,
+            (CASE
              WHEN s.account_id = :recipient_id THEN
                EXISTS (
                  SELECT *
@ -84,7 +86,8 @@ class NotifyService < BaseService
                )
              ELSE
                FALSE
-             END)
+             END),
+            ARRAY[s.id]
          FROM statuses s
          WHERE s.id = :id
        UNION ALL
@ -100,10 +103,11 @@ class NotifyService < BaseService
                )
              ELSE
                FALSE
-             END)
+             END),
+            st.path || s.id
          FROM ancestors st
          JOIN statuses s ON s.id = st.in_reply_to_id
-          WHERE st.replying_to_sender IS FALSE
+          WHERE st.replying_to_sender IS FALSE AND NOT s.id = ANY(path)
      )
      SELECT COUNT(*)
      FROM ancestors st
--- a/app/views/admin/reports/show.html.haml
+++ b/app/views/admin/reports/show.html.haml
@ -92,7 +92,7 @@
 %hr.spacer

 .speech-bubble
-  .speech-bubble__bubble= simple_format(@report.comment.presence || t('admin.reports.comment.none'))
+  .speech-bubble__bubble= simple_format(h(@report.comment.presence || t('admin.reports.comment.none')))
  .speech-bubble__owner
    - if @report.account.local?
      = admin_account_link_to @report.account
--- a/chart/values.yaml
+++ b/chart/values.yaml
@ -8,7 +8,7 @@ image:
  # built from the most recent commit
  #
  # tag: latest
-  tag: v3.3.0
+  tag: v3.4.6
  # use `Always` when using `latest` tag
  pullPolicy: IfNotPresent

--- a/config/brakeman.ignore
+++ b/config/brakeman.ignore
@ -60,46 +60,6 @@
      "confidence": "High",
      "note": ""
    },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "6e4051854bb62e2ddbc671f82d6c2328892e1134b8b28105ecba9b0122540714",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/account.rb",
-      "line": 479,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "find_by_sql([\"          WITH first_degree AS (\\n            SELECT target_account_id\\n            FROM follows\\n            WHERE account_id = ?\\n            UNION ALL\\n            SELECT ?\\n          )\\n          SELECT\\n            accounts.*,\\n            (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n          FROM accounts\\n          LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?)\\n          WHERE accounts.id IN (SELECT * FROM first_degree)\\n            AND #{query} @@ #{textsearch}\\n            AND accounts.suspended_at IS NULL\\n            AND accounts.moved_to_account_id IS NULL\\n          GROUP BY accounts.id\\n          ORDER BY rank DESC\\n          LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, account.id, limit, offset])",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Account",
-        "method": "advanced_search_for"
-      },
-      "user_input": "textsearch",
-      "confidence": "Medium",
-      "note": ""
-    },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "6f075c1484908e3ec9bed21ab7cf3c7866be8da3881485d1c82e13093aefcbd7",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/status.rb",
-      "line": 105,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "result.joins(\"LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Status",
-        "method": null
-      },
-      "user_input": "id",
-      "confidence": "Weak",
-      "note": ""
-    },
    {
      "warning_type": "Mass Assignment",
      "warning_code": 105,
@ -140,26 +100,6 @@
      "confidence": "High",
      "note": ""
    },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "9251d682c4e2840e1b2fea91e7d758efe2097ecb7f6255c065e3750d25eb178c",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/account.rb",
-      "line": 448,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "find_by_sql([\"        SELECT\\n          accounts.*,\\n          ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n        FROM accounts\\n        WHERE #{query} @@ #{textsearch}\\n          AND accounts.suspended_at IS NULL\\n          AND accounts.moved_to_account_id IS NULL\\n        ORDER BY rank DESC\\n        LIMIT ? OFFSET ?\\n\".squish, limit, offset])",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Account",
-        "method": "search_for"
-      },
-      "user_input": "textsearch",
-      "confidence": "Medium",
-      "note": ""
-    },
    {
      "warning_type": "SQL Injection",
      "warning_code": 0,
@ -220,26 +160,6 @@
      "confidence": "High",
      "note": ""
    },
-    {
-      "warning_type": "SQL Injection",
-      "warning_code": 0,
-      "fingerprint": "e21d8fee7a5805761679877ca35ed1029c64c45ef3b4012a30262623e1ba8bb9",
-      "check_name": "SQL",
-      "message": "Possible SQL injection",
-      "file": "app/models/account.rb",
-      "line": 495,
-      "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
-      "code": "find_by_sql([\"          SELECT\\n            accounts.*,\\n            (count(f.id) + 1) * ts_rank_cd(#{textsearch}, #{query}, 32) AS rank\\n          FROM accounts\\n          LEFT OUTER JOIN follows AS f ON (accounts.id = f.account_id AND f.target_account_id = ?) OR (accounts.id = f.target_account_id AND f.account_id = ?)\\n          WHERE #{query} @@ #{textsearch}\\n            AND accounts.suspended_at IS NULL\\n            AND accounts.moved_to_account_id IS NULL\\n          GROUP BY accounts.id\\n          ORDER BY rank DESC\\n          LIMIT ? OFFSET ?\\n\".squish, account.id, account.id, limit, offset])",
-      "render_path": null,
-      "location": {
-        "type": "method",
-        "class": "Account",
-        "method": "advanced_search_for"
-      },
-      "user_input": "textsearch",
-      "confidence": "Medium",
-      "note": ""
-    },
    {
      "warning_type": "Mass Assignment",
      "warning_code": 105,
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@ -118,7 +118,7 @@ Rails.application.configure do
    'Server'                 => 'Mastodon',
    'X-Frame-Options'        => 'DENY',
    'X-Content-Type-Options' => 'nosniff',
-    'X-XSS-Protection'       => '1; mode=block',
+    'X-XSS-Protection'       => '0',
    'Permissions-Policy'     => 'interest-cohort=()',
  }

--- a/docker-compose.yml
+++ b/docker-compose.yml
@ -43,7 +43,7 @@ services:

  web:
    build: .
-    image: tootsuite/mastodon:v3.4.5
+    image: tootsuite/mastodon:v3.4.6
    restart: always
    env_file: .env.production
    command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000"
@ -63,7 +63,7 @@ services:

  streaming:
    build: .
-    image: tootsuite/mastodon:v3.4.5
+    image: tootsuite/mastodon:v3.4.6
    restart: always
    env_file: .env.production
    command: node ./streaming
@ -80,7 +80,7 @@ services:

  sidekiq:
    build: .
-    image: tootsuite/mastodon:v3.4.5
+    image: tootsuite/mastodon:v3.4.6
    restart: always
    env_file: .env.production
    command: bundle exec sidekiq
--- a/lib/mastodon/version.rb
+++ b/lib/mastodon/version.rb
@ -13,7 +13,7 @@ module Mastodon
    end

    def patch
-      5
+      6
    end

    def flags
--- a/lib/tasks/mastodon.rake
+++ b/lib/tasks/mastodon.rake
@ -441,7 +441,7 @@ namespace :mastodon do

  namespace :webpush do
    desc 'Generate VAPID key'
-    task generate_vapid_key: :environment do
+    task :generate_vapid_key do
      vapid_key = Webpush.generate_key
      puts "VAPID_PRIVATE_KEY=#{vapid_key.private_key}"
      puts "VAPID_PUBLIC_KEY=#{vapid_key.public_key}"
--- a/spec/helpers/jsonld_helper_spec.rb
+++ b/spec/helpers/jsonld_helper_spec.rb
@ -89,4 +89,86 @@ describe JsonLdHelper do
      expect(fetch_resource_without_id_validation('https://host.test/')).to eq({})
    end
  end
+
+  context 'compaction and forwarding' do
+    let(:json) do
+      {
+        '@context' => [
+          'https://www.w3.org/ns/activitystreams',
+          'https://w3id.org/security/v1',
+          {
+            'obsolete' => 'http://ostatus.org#',
+            'convo' => 'obsolete:conversation',
+            'new' => 'https://obscure-unreleased-test.joinmastodon.org/#',
+          },
+        ],
+        'type' => 'Create',
+        'to' => ['https://www.w3.org/ns/activitystreams#Public'],
+        'object' => {
+          'id' => 'https://example.com/status',
+          'type' => 'Note',
+          'inReplyTo' => nil,
+          'convo' => 'https://example.com/conversation',
+          'tag' => [
+            {
+              'type' => 'Mention',
+              'href' => ['foo'],
+            }
+          ],
+        },
+        'signature' => {
+          'type' => 'RsaSignature2017',
+          'created' => '2022-02-02T12:00:00Z',
+          'creator' => 'https://example.com/actor#main-key',
+          'signatureValue' => 'some-sig',
+        },
+      }
+    end
+
+    describe '#compact' do
+      it 'properly compacts JSON-LD with alternative context definitions' do
+        expect(compact(json).dig('object', 'conversation')).to eq 'https://example.com/conversation'
+      end
+
+      it 'compacts single-item arrays' do
+        expect(compact(json).dig('object', 'tag', 'href')).to eq 'foo'
+      end
+
+      it 'compacts the activistreams Public collection' do
+        expect(compact(json)['to']).to eq 'as:Public'
+      end
+
+      it 'properly copies signature' do
+        expect(compact(json)['signature']).to eq json['signature']
+      end
+    end
+
+    describe 'patch_for_forwarding!' do
+      it 'properly patches incompatibilities' do
+        json['object'].delete('convo')
+        compacted = compact(json)
+        patch_for_forwarding!(json, compacted)
+        expect(compacted['to']).to eq ['https://www.w3.org/ns/activitystreams#Public']
+        expect(compacted.dig('object', 'tag', 0, 'href')).to eq ['foo']
+        expect(safe_for_forwarding?(json, compacted)).to eq true
+      end
+    end
+
+    describe 'safe_for_forwarding?' do
+      it 'deems a safe compacting as such' do
+        json['object'].delete('convo')
+        compacted = compact(json)
+        deemed_compatible = patch_for_forwarding!(json, compacted)
+        expect(compacted['to']).to eq ['https://www.w3.org/ns/activitystreams#Public']
+        expect(safe_for_forwarding?(json, compacted)).to eq true
+      end
+
+      it 'deems an unsafe compacting as such' do
+        compacted = compact(json)
+        deemed_compatible = patch_for_forwarding!(json, compacted)
+        expect(compacted['to']).to eq ['https://www.w3.org/ns/activitystreams#Public']
+        expect(safe_for_forwarding?(json, compacted)).to eq false
+      end
+    end
+  end
 end
--- a/spec/models/status_spec.rb
+++ b/spec/models/status_spec.rb
@ -267,6 +267,87 @@ RSpec.describe Status, type: :model do
    end
  end

+  describe '.tagged_with' do
+    let(:tag1)     { Fabricate(:tag) }
+    let(:tag2)     { Fabricate(:tag) }
+    let(:tag3)     { Fabricate(:tag) }
+    let!(:status1) { Fabricate(:status, tags: [tag1]) }
+    let!(:status2) { Fabricate(:status, tags: [tag2]) }
+    let!(:status3) { Fabricate(:status, tags: [tag3]) }
+    let!(:status4) { Fabricate(:status, tags: []) }
+    let!(:status5) { Fabricate(:status, tags: [tag1, tag2, tag3]) }
+
+    context 'when given one tag' do
+      it 'returns the expected statuses' do
+        expect(Status.tagged_with([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status5.id]
+        expect(Status.tagged_with([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status5.id]
+        expect(Status.tagged_with([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id, status5.id]
+      end
+    end
+
+    context 'when given multiple tags' do
+      it 'returns the expected statuses' do
+        expect(Status.tagged_with([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status2.id, status5.id]
+        expect(Status.tagged_with([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status3.id, status5.id]
+        expect(Status.tagged_with([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status3.id, status5.id]
+      end
+    end
+  end
+
+  describe '.tagged_with_all' do
+    let(:tag1)     { Fabricate(:tag) }
+    let(:tag2)     { Fabricate(:tag) }
+    let(:tag3)     { Fabricate(:tag) }
+    let!(:status1) { Fabricate(:status, tags: [tag1]) }
+    let!(:status2) { Fabricate(:status, tags: [tag2]) }
+    let!(:status3) { Fabricate(:status, tags: [tag3]) }
+    let!(:status4) { Fabricate(:status, tags: []) }
+    let!(:status5) { Fabricate(:status, tags: [tag1, tag2]) }
+
+    context 'when given one tag' do
+      it 'returns the expected statuses' do
+        expect(Status.tagged_with_all([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status5.id]
+        expect(Status.tagged_with_all([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status5.id]
+        expect(Status.tagged_with_all([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id]
+      end
+    end
+
+    context 'when given multiple tags' do
+      it 'returns the expected statuses' do
+        expect(Status.tagged_with_all([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status5.id]
+        expect(Status.tagged_with_all([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq []
+        expect(Status.tagged_with_all([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq []
+      end
+    end
+  end
+
+  describe '.tagged_with_none' do
+    let(:tag1)     { Fabricate(:tag) }
+    let(:tag2)     { Fabricate(:tag) }
+    let(:tag3)     { Fabricate(:tag) }
+    let!(:status1) { Fabricate(:status, tags: [tag1]) }
+    let!(:status2) { Fabricate(:status, tags: [tag2]) }
+    let!(:status3) { Fabricate(:status, tags: [tag3]) }
+    let!(:status4) { Fabricate(:status, tags: []) }
+    let!(:status5) { Fabricate(:status, tags: [tag1, tag2, tag3]) }
+
+    context 'when given one tag' do
+      it 'returns the expected statuses' do
+        expect(Status.tagged_with_none([tag1.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status3.id, status4.id]
+        expect(Status.tagged_with_none([tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status3.id, status4.id]
+        expect(Status.tagged_with_none([tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status2.id, status4.id]
+      end
+    end
+
+    context 'when given multiple tags' do
+      it 'returns the expected statuses' do
+        expect(Status.tagged_with_none([tag1.id, tag2.id]).reorder(:id).pluck(:id).uniq).to eq [status3.id, status4.id]
+        expect(Status.tagged_with_none([tag1.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status2.id, status4.id]
+        expect(Status.tagged_with_none([tag2.id, tag3.id]).reorder(:id).pluck(:id).uniq).to eq [status1.id, status4.id]
+      end
+    end
+  end
+
  describe '.permitted_for' do
    subject { described_class.permitted_for(target_account, account).pluck(:visibility) }
Author	SHA1	Message	Date
ida schmidt	f3cae1d606	Merge tag 'v3.4.6' of https://github.com/tootsuite/mastodon	2022-02-03 06:47:40 -07:00
Claire	93a6c143af	Fix insufficient sanitization of report comments (#17430 )	2022-02-03 14:08:24 +01:00
Claire	bb7b2868a0	Bump version to 3.4.6	2022-02-02 23:48:38 +01:00
Wonderfall	a06dda41d0	disable legacy XSS filtering (#17289 ) Browsers are phasing out X-XSS-Protection, but Safari and IE still support it.	2022-02-02 23:30:15 +01:00
Claire	bf005edd30	Change mastodon:webpush:generate_vapid_key task to not require functional env (#17338 ) Fixes #17297	2022-02-02 23:30:15 +01:00
Claire	df68d2eab8	Fix response_to_recipient? CTE	2022-02-02 23:30:15 +01:00
Claire	b27f50da5a	Fix insufficient sanitization of report comments	2022-02-02 23:30:15 +01:00
Claire	e2009ced3a	Fix compacted JSON-LD possibly causing compatibility issues on forwarding	2022-02-02 23:30:15 +01:00
Puck Meerburg	fe0210074f	Compact JSON-LD signed incoming activities	2022-02-02 23:30:15 +01:00
Claire	c8dbbd60eb	Fix error-prone SQL queries (#15828 ) * Fix error-prone SQL queries in Account search While this code seems to not present an actual vulnerability, one could easily be introduced by mistake due to how the query is built. This PR parameterises the `to_tsquery` input to make the query more robust. * Harden code for Status#tagged_with_all and Status#tagged_with_none Those two scopes aren't used in a way that could be vulnerable to an SQL injection, but keeping them unchanged might be a hazard. * Remove unneeded spaces surrounding tsquery term * Please CodeClimate * Move advanced_search_for SQL template to its own function This avoids one level of indentation while making clearer that the SQL template isn't build from all the dynamic parameters of advanced_search_for. * Add tests covering tagged_with, tagged_with_all and tagged_with_none * Rewrite tagged_with_none to avoid multiple joins and make it more robust * Remove obsolete brakeman warnings * Revert "Remove unneeded spaces surrounding tsquery term" The two queries are not strictly equivalent. This reverts commit 86f16c537e06c6ba4a8b250f25dcce9f049023ff.	2022-02-02 23:30:15 +01:00
Claire	6d831fe274	Fix spurious errors when receiving an Add activity for a private post (#17425 )	2022-02-02 22:59:34 +01:00