2021-02-03 security fixes

Merge remote-tracking branch 'glitch/main'

6 files changed, 165 insertions, 56 deletions

diff --git a/app/helpers/context_helper.rb b/app/helpers/context_helper.rb
new file mode 100644
index 000000000..2f5fecaae
--- /dev/null
+++ 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 = {
+    direct_message: { 'litepub' => 'http://litepub.social/ns#', 'directMessage' => 'litepub:directMessage' },
+    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' } },
+    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
diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index c24d2ddf1..c6557817d 100644
--- 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
@@ -69,6 +71,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)
diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb
index d8b0c63b2..098b6296f 100644
--- 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 = {
-    direct_message: { 'litepub': 'http://litepub.social/ns#', 'directMessage': 'litepub:directMessage' },
-    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' } },
-    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
diff --git a/app/models/user.rb b/app/models/user.rb
index e47b5f135..9afdc481d 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -45,6 +45,8 @@ class User < ApplicationRecord
   self.ignored_columns = %w(
     remember_created_at
     remember_token
+    current_sign_in_ip
+    last_sign_in_ip
   )
 
   include Settings::Extend
diff --git a/app/services/activitypub/process_collection_service.rb b/app/services/activitypub/process_collection_service.rb
index 170e6709c..eb008c40a 100644
--- 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']
diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb
index 09e28b76b..0f3516d28 100644
--- 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