Address vulnerability from GHSA-3fjr-858r-92rw

Based on https://github.com/mastodon/mastodon/compare/v4.1.12...v4.1.13

Notes:

* spec/lib/linked_data_signature_spec.rb: missing code this patch
  addresses; ignored

diff --git a/app/controllers/concerns/signature_verification.rb b/app/controllers/concerns/signature_verification.rb
index 2394574b3bede0f8c5de5edcea8f3c59070e4a2d..ecdccad4f7b96f00b80696319642b4ee36b8375a 100644 (file)
--- a/app/controllers/concerns/signature_verification.rb
+++ b/app/controllers/concerns/signature_verification.rb
@@ -246,7 +246,7 @@ module SignatureVerification
       stoplight_wrap_request { ResolveAccountService.new.call(key_id.gsub(/\Aacct:/, ''), suppress_errors: false) }
     elsif !ActivityPub::TagManager.instance.local_uri?(key_id)
       account   = ActivityPub::TagManager.instance.uri_to_actor(key_id)
-      account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, id: false, suppress_errors: false) }
+      account ||= stoplight_wrap_request { ActivityPub::FetchRemoteKeyService.new.call(key_id, suppress_errors: false) }
       account
     end
   rescue Mastodon::PrivateNetworkAddressError => e

diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index 102e4b13281ad74a4367a47c68d8178a898057b5..18314d99f0a4a465b5beea6d5c9afccfc80e48a8 100644 (file)
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@@ -157,8 +157,8 @@ module JsonLdHelper
     end
   end
 
-  def fetch_resource(uri, id, on_behalf_of = nil)
-    unless id
+  def fetch_resource(uri, id_is_known, on_behalf_of = nil)
+    unless id_is_known
       json = fetch_resource_without_id_validation(uri, on_behalf_of)
 
       return if !json.is_a?(Hash) || unsupported_uri_scheme?(json['id'])

diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb
index f4c67cccd733a77d21f2a45658d3ab8dfef9aba0..f2b178016cd5e5ea008803460559f3d01df55d0b 100644 (file)
--- a/app/lib/activitypub/activity.rb
+++ b/app/lib/activitypub/activity.rb
@@ -152,7 +152,9 @@ class ActivityPub::Activity
   def fetch_remote_original_status
     if object_uri.start_with?('http')
       return if ActivityPub::TagManager.instance.local_uri?(object_uri)
-      ActivityPub::FetchRemoteStatusService.new.call(object_uri, id: true, on_behalf_of: @account.followers.local.first)
+
+      # in 4.1.12 there is an extra argument `request_id: @options[:request_id]` here
+      ActivityPub::FetchRemoteStatusService.new.call(object_uri, on_behalf_of: @account.followers.local.first)
     elsif @object['url'].present?
       ::FetchRemoteStatusService.new.call(@object['url'])
     end

diff --git a/app/lib/activitypub/linked_data_signature.rb b/app/lib/activitypub/linked_data_signature.rb
index f90adaf6c5fd3be5cb92b4276415ce6d02e9b5dd..9c2618515fde40827b89ed3b55ebf21198a04cfa 100644 (file)
--- a/app/lib/activitypub/linked_data_signature.rb
+++ b/app/lib/activitypub/linked_data_signature.rb
@@ -19,7 +19,8 @@ class ActivityPub::LinkedDataSignature
     return unless type == 'RsaSignature2017'
 
     creator   = ActivityPub::TagManager.instance.uri_to_actor(creator_uri)
-    creator ||= ActivityPub::FetchRemoteKeyService.new.call(creator_uri, id: false)
+    # 4.1.12 uses a reassignment `if creator&.public_key.blank?`
+    creator ||= ActivityPub::FetchRemoteKeyService.new.call(creator_uri)
 
     return if creator.nil?
 

diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb
index ca7a8c6ca8930bff524c15cfdc85ca8fa73e6489..744228645e6b78842141d5cfb339ccb51554139d 100644 (file)
--- a/app/services/activitypub/fetch_remote_account_service.rb
+++ b/app/services/activitypub/fetch_remote_account_service.rb
@@ -2,7 +2,7 @@
 
 class ActivityPub::FetchRemoteAccountService < ActivityPub::FetchRemoteActorService
   # Does a WebFinger roundtrip on each call, unless `only_key` is true
-  def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true)
+  def call(uri, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true)
     actor = super
     return actor if actor.nil? || actor.is_a?(Account)
 

diff --git a/app/services/activitypub/fetch_remote_actor_service.rb b/app/services/activitypub/fetch_remote_actor_service.rb
index 17bf2f2876ac97ae688d7523e0b4c6f7ebf6547c..81815bcddad62f8a31de54944297bd699d2f9686 100644 (file)
--- a/app/services/activitypub/fetch_remote_actor_service.rb
+++ b/app/services/activitypub/fetch_remote_actor_service.rb
@@ -10,15 +10,15 @@ class ActivityPub::FetchRemoteActorService < BaseService
   SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze
 
   # Does a WebFinger roundtrip on each call, unless `only_key` is true
-  def call(uri, id: true, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true)
+  def call(uri, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true)
     return if domain_not_allowed?(uri)
     return ActivityPub::TagManager.instance.uri_to_actor(uri) if ActivityPub::TagManager.instance.local_uri?(uri)
 
     @json = begin
       if prefetched_body.nil?
-        fetch_resource(uri, id)
+        fetch_resource(uri, true)
       else
-        body_to_json(prefetched_body, compare_id: id ? uri : nil)
+        body_to_json(prefetched_body, compare_id: uri)
       end
     rescue Oj::ParseError
       raise Error, "Error parsing JSON-LD document #{uri}"

diff --git a/app/services/activitypub/fetch_remote_key_service.rb b/app/services/activitypub/fetch_remote_key_service.rb
index 32e82b47a58d67e0ca179ce70b6938e31eab2cdc..1f671849236cd80499851786b3dc6bf1d9b4950d 100644 (file)
--- a/app/services/activitypub/fetch_remote_key_service.rb
+++ b/app/services/activitypub/fetch_remote_key_service.rb
@@ -6,23 +6,10 @@ class ActivityPub::FetchRemoteKeyService < BaseService
   class Error < StandardError; end
 
   # Returns actor that owns the key
-  def call(uri, id: true, prefetched_body: nil, suppress_errors: true)
+  def call(uri, suppress_errors: true)
     raise Error, 'No key URI given' if uri.blank?
 
-    if prefetched_body.nil?
-      if id
-        @json = fetch_resource_without_id_validation(uri)
-        if actor_type?
-          @json = fetch_resource(@json['id'], true)
-        elsif uri != @json['id']
-          raise Error, "Fetched URI #{uri} has wrong id #{@json['id']}"
-        end
-      else
-        @json = fetch_resource(uri, id)
-      end
-    else
-      @json = body_to_json(prefetched_body, compare_id: id ? uri : nil)
-    end
+    @json = fetch_resource(uri, false)
 
     raise Error, "Unable to fetch key JSON at #{uri}" if @json.nil?
     raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context?(@json)

diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb
index 80309824509a4746a093555b4d1f92a2e38fcdc2..d9227d300ef29884afc0da4d44a5521b21ca64d7 100644 (file)
--- a/app/services/activitypub/fetch_remote_status_service.rb
+++ b/app/services/activitypub/fetch_remote_status_service.rb
@@ -4,12 +4,12 @@ class ActivityPub::FetchRemoteStatusService < BaseService
   include JsonLdHelper
 
   # Should be called when uri has already been checked for locality
-  def call(uri, id: true, prefetched_body: nil, on_behalf_of: nil)
+  def call(uri, prefetched_body: nil, on_behalf_of: nil)
     @json = begin
       if prefetched_body.nil?
-        fetch_resource(uri, id, on_behalf_of)
+        fetch_resource(uri, true, on_behalf_of)
       else
-        body_to_json(prefetched_body, compare_id: id ? uri : nil)
+        body_to_json(prefetched_body, compare_id: uri)
       end
     end
 
@@ -52,7 +52,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
 
   def account_from_uri(uri)
     actor = ActivityPub::TagManager.instance.uri_to_resource(uri, Account)
-    actor = ActivityPub::FetchRemoteAccountService.new.call(uri, id: true) if actor.nil? || actor.possibly_stale?
+    actor = ActivityPub::FetchRemoteAccountService.new.call(uri) if actor.nil? || actor.possibly_stale?
     actor
   end
 

diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb
index 456b3524b5dfe7320488c6971342594d376966f7..6f6b187805dbf7067505c38aeee5cd60bcfce655 100644 (file)
--- a/app/services/activitypub/process_account_service.rb
+++ b/app/services/activitypub/process_account_service.rb
@@ -249,7 +249,7 @@ class ActivityPub::ProcessAccountService < BaseService
 
   def moved_account
     account   = ActivityPub::TagManager.instance.uri_to_resource(@json['movedTo'], Account)
-    account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], id: true, break_on_redirect: true)
+    account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], break_on_redirect: true)
     account
   end
 

diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb
index 73204e55db0c41dac24f31885d0b1c75d4a6092a..e1af234785211138e2e40ff70cb77f972d4460ef 100644 (file)
--- a/app/services/fetch_resource_service.rb
+++ b/app/services/fetch_resource_service.rb
@@ -47,7 +47,15 @@ class FetchResourceService < BaseService
       body = response.body_with_limit
       json = body_to_json(body)
 
-      [json['id'], { prefetched_body: body, id: true }] if supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) || expected_type?(json))
+      return unless supported_context?(json) && (equals_or_includes_any?(json['type'], ActivityPub::FetchRemoteActorService::SUPPORTED_TYPES) || expected_type?(json))
+
+      if json['id'] != @url
+        return if terminal
+
+        return process(json['id'], terminal: true)
+      end
+
+      [@url, { prefetched_body: body }]
     elsif !terminal
       link_header = response['Link'] && parse_link_header(response)
 

diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb
index ec6f1f41d8f6cedd13519f748743e356010293e7..9ee2c15248069d65fe091ac4968af71d57c48d03 100644 (file)
--- a/spec/services/activitypub/fetch_remote_account_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb
@@ -16,7 +16,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
   end
 
   describe '#call' do
-    let(:account) { subject.call('https://example.com/alice', id: true) }
+    let(:account) { subject.call('https://example.com/alice') }
 
     shared_examples 'sets profile data' do
       it 'returns an account' do

diff --git a/spec/services/activitypub/fetch_remote_actor_service_spec.rb b/spec/services/activitypub/fetch_remote_actor_service_spec.rb
index 20117c66d047642b673f7197d7ff442faecce9c4..56805b33254760ad66abb4f523444192d9c97b84 100644 (file)
--- a/spec/services/activitypub/fetch_remote_actor_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_actor_service_spec.rb
@@ -16,7 +16,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService, type: :service do
   end
 
   describe '#call' do
-    let(:account) { subject.call('https://example.com/alice', id: true) }
+    let(:account) { subject.call('https://example.com/alice') }
 
     shared_examples 'sets profile data' do
       it 'returns an account' do

diff --git a/spec/services/activitypub/fetch_remote_key_service_spec.rb b/spec/services/activitypub/fetch_remote_key_service_spec.rb
index 3186c4270d7e3da6ed03933a9041777d0e24c5a0..9c818d12c0d6346d83126b1e7dacf17be5c99b4e 100644 (file)
--- a/spec/services/activitypub/fetch_remote_key_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_key_service_spec.rb
@@ -43,7 +43,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService, type: :service do
   end
 
   describe '#call' do
-    let(:account) { subject.call(public_key_id, id: false) }
+    let(:account) { subject.call(public_key_id) }
 
     context 'when the key is a sub-object from the actor' do
       before do

diff --git a/spec/services/fetch_resource_service_spec.rb b/spec/services/fetch_resource_service_spec.rb
index c0c96ab69c95f650116abc861b051cf9b536237c..081dab408a0af82241b30e66e54c9de8f7353485 100644 (file)
--- a/spec/services/fetch_resource_service_spec.rb
+++ b/spec/services/fetch_resource_service_spec.rb
@@ -54,7 +54,7 @@ RSpec.describe FetchResourceService, type: :service do
 
       let(:json) do
         {
-          id: 1,
+          id: 'http://example.com/foo',
           '@context': ActivityPub::TagManager::CONTEXT,
           type: 'Note',
         }.to_json
@@ -79,14 +79,14 @@ RSpec.describe FetchResourceService, type: :service do
         let(:content_type) { 'application/activity+json; charset=utf-8' }
         let(:body) { json }
 
-        it { is_expected.to eq [1, { prefetched_body: body, id: true }] }
+        it { is_expected.to eq ['http://example.com/foo', { prefetched_body: body }] }
       end
 
       context 'when content type is ld+json with profile' do
         let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }
         let(:body) { json }
 
-        it { is_expected.to eq [1, { prefetched_body: body, id: true }] }
+        it { is_expected.to eq ['http://example.com/foo', { prefetched_body: body }] }
       end
 
       before do
@@ -104,7 +104,7 @@ RSpec.describe FetchResourceService, type: :service do
         let(:content_type) { 'text/html' }
         let(:body) { '<html><head><link rel="alternate" href="http://example.com/foo" type="application/activity+json"/></head></html>' }
 
-        it { is_expected.to eq [1, { prefetched_body: json, id: true }] }
+        it { is_expected.to eq ['http://example.com/foo', { prefetched_body: json }] }
       end
     end
   end

diff --git a/spec/services/resolve_url_service_spec.rb b/spec/services/resolve_url_service_spec.rb
index b3e3defbff2ccd942f847d6e6dd1db9ef4b50631..5b6b7b9a66a0f2df7bbd6b586a6f42bd9ad3886c 100644 (file)
--- a/spec/services/resolve_url_service_spec.rb
+++ b/spec/services/resolve_url_service_spec.rb
@@ -139,6 +139,7 @@ describe ResolveURLService, type: :service do
         stub_request(:get, url).to_return(status: 302, headers: { 'Location' => status_url })
         body = ActiveModelSerializers::SerializableResource.new(status, serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter).to_json
         stub_request(:get, status_url).to_return(body: body, headers: { 'Content-Type' => 'application/activity+json' })
+        stub_request(:get, uri).to_return(body: body, headers: { 'Content-Type' => 'application/activity+json' })
       end
 
       it 'returns status by url' do