From c397c1a9e3090a0e879c15cafd48d5f1414b6836 Mon Sep 17 00:00:00 2001
From: Claire <claire.github-309c@sitedethib.com>
Date: Fri, 16 Feb 2024 11:56:12 +0100
Subject: [PATCH] Merge pull request from GHSA-jhrq-qvrm-qr36

* Fix insufficient Content-Type checking of fetched ActivityStreams objects

* Allow JSON-LD documents with multiple profiles
---
 app/helpers/jsonld_helper.rb                       | 14 +++++++++++++-
 app/services/fetch_resource_service.rb             |  2 +-
 spec/helpers/jsonld_helper_spec.rb                 | 14 +++++++-------
 spec/lib/activitypub/activity/announce_spec.rb     |  4 ++--
 .../fetch_featured_collection_service_spec.rb      | 12 ++++++------
 .../fetch_remote_account_service_spec.rb           |  6 +++---
 .../activitypub/fetch_replies_service_spec.rb      |  6 +++---
 .../synchronize_followers_service_spec.rb          |  6 +++---
 .../activitypub/fetch_replies_worker_spec.rb       |  2 +-
 9 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index 18314d99f0..332e623f31 100644
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@@ -176,7 +176,19 @@ module JsonLdHelper
     build_request(uri, on_behalf_of).perform do |response|
       raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) || !raise_on_temporary_error
 
-      body_to_json(response.body_with_limit) if response.code == 200
+      body_to_json(response.body_with_limit) if response.code == 200 && valid_activitypub_content_type?(response)
+    end
+  end
+
+  def valid_activitypub_content_type?(response)
+    return true if response.mime_type == 'application/activity+json'
+
+    # When the mime type is `application/ld+json`, we need to check the profile,
+    # but `http.rb` does not parse it for us.
+    return false unless response.mime_type == 'application/ld+json'
+
+    response.headers[HTTP::Headers::CONTENT_TYPE]&.split(';')&.map(&:strip)&.any? do |str|
+      str.start_with?('profile="') && str[9...-1].split.include?('https://www.w3.org/ns/activitystreams')
     end
   end
 
diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb
index b012880c7a..d6fd63eca0 100644
--- a/app/services/fetch_resource_service.rb
+++ b/app/services/fetch_resource_service.rb
@@ -43,7 +43,7 @@ class FetchResourceService < BaseService
     @response_code = response.code
     return nil if response.code != 200
 
-    if ['application/activity+json', 'application/ld+json'].include?(response.mime_type)
+    if valid_activitypub_content_type?(response)
       body = response.body_with_limit
       json = body_to_json(body)
 
diff --git a/spec/helpers/jsonld_helper_spec.rb b/spec/helpers/jsonld_helper_spec.rb
index 744a14f260..54355b8482 100644
--- a/spec/helpers/jsonld_helper_spec.rb
+++ b/spec/helpers/jsonld_helper_spec.rb
@@ -56,15 +56,15 @@ describe JsonLdHelper do
   describe '#fetch_resource' do
     context 'when the second argument is false' do
       it 'returns resource even if the retrieved ID and the given URI does not match' do
-        stub_request(:get, 'https://bob.test/').to_return body: '{"id": "https://alice.test/"}'
-        stub_request(:get, 'https://alice.test/').to_return body: '{"id": "https://alice.test/"}'
+        stub_request(:get, 'https://bob.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })
+        stub_request(:get, 'https://alice.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })
 
         expect(fetch_resource('https://bob.test/', false)).to eq({ 'id' => 'https://alice.test/' })
       end
 
       it 'returns nil if the object identified by the given URI and the object identified by the retrieved ID does not match' do
-        stub_request(:get, 'https://mallory.test/').to_return body: '{"id": "https://marvin.test/"}'
-        stub_request(:get, 'https://marvin.test/').to_return body: '{"id": "https://alice.test/"}'
+        stub_request(:get, 'https://mallory.test/').to_return(body: '{"id": "https://marvin.test/"}', headers: { 'Content-Type': 'application/activity+json' })
+        stub_request(:get, 'https://marvin.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })
 
         expect(fetch_resource('https://mallory.test/', false)).to eq nil
       end
@@ -72,7 +72,7 @@ describe JsonLdHelper do
 
     context 'when the second argument is true' do
       it 'returns nil if the retrieved ID and the given URI does not match' do
-        stub_request(:get, 'https://mallory.test/').to_return body: '{"id": "https://alice.test/"}'
+        stub_request(:get, 'https://mallory.test/').to_return(body: '{"id": "https://alice.test/"}', headers: { 'Content-Type': 'application/activity+json' })
         expect(fetch_resource('https://mallory.test/', true)).to eq nil
       end
     end
@@ -80,12 +80,12 @@ describe JsonLdHelper do
 
   describe '#fetch_resource_without_id_validation' do
     it 'returns nil if the status code is not 200' do
-      stub_request(:get, 'https://host.test/').to_return status: 400, body: '{}'
+      stub_request(:get, 'https://host.test/').to_return(status: 400, body: '{}', headers: { 'Content-Type': 'application/activity+json' })
       expect(fetch_resource_without_id_validation('https://host.test/')).to eq nil
     end
 
     it 'returns hash' do
-      stub_request(:get, 'https://host.test/').to_return status: 200, body: '{}'
+      stub_request(:get, 'https://host.test/').to_return(status: 200, body: '{}', headers: { 'Content-Type': 'application/activity+json' })
       expect(fetch_resource_without_id_validation('https://host.test/')).to eq({})
     end
   end
diff --git a/spec/lib/activitypub/activity/announce_spec.rb b/spec/lib/activitypub/activity/announce_spec.rb
index 41806b2582..67a9943468 100644
--- a/spec/lib/activitypub/activity/announce_spec.rb
+++ b/spec/lib/activitypub/activity/announce_spec.rb
@@ -33,7 +33,7 @@ RSpec.describe ActivityPub::Activity::Announce do
     context 'when sender is followed by a local account' do
       before do
         Fabricate(:account).follow!(sender)
-        stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json))
+        stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json), headers: { 'Content-Type': 'application/activity+json' })
         subject.perform
       end
 
@@ -118,7 +118,7 @@ RSpec.describe ActivityPub::Activity::Announce do
       subject { described_class.new(json, sender, relayed_through_account: relay_account) }
 
       before do
-        stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json))
+        stub_request(:get, 'https://example.com/actor/hello-world').to_return(body: Oj.dump(unknown_object_json), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       context 'and the relay is enabled' do
diff --git a/spec/services/activitypub/fetch_featured_collection_service_spec.rb b/spec/services/activitypub/fetch_featured_collection_service_spec.rb
index f552b9dc07..e7351088ba 100644
--- a/spec/services/activitypub/fetch_featured_collection_service_spec.rb
+++ b/spec/services/activitypub/fetch_featured_collection_service_spec.rb
@@ -60,10 +60,10 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do
 
   shared_examples 'sets pinned posts' do
     before do
-      stub_request(:get, 'https://example.com/account/pinned/1').to_return(status: 200, body: Oj.dump(status_json_1))
-      stub_request(:get, 'https://example.com/account/pinned/2').to_return(status: 200, body: Oj.dump(status_json_2))
+      stub_request(:get, 'https://example.com/account/pinned/1').to_return(status: 200, body: Oj.dump(status_json_1), headers: { 'Content-Type': 'application/activity+json' })
+      stub_request(:get, 'https://example.com/account/pinned/2').to_return(status: 200, body: Oj.dump(status_json_2), headers: { 'Content-Type': 'application/activity+json' })
       stub_request(:get, 'https://example.com/account/pinned/3').to_return(status: 404)
-      stub_request(:get, 'https://example.com/account/pinned/4').to_return(status: 200, body: Oj.dump(status_json_4))
+      stub_request(:get, 'https://example.com/account/pinned/4').to_return(status: 200, body: Oj.dump(status_json_4), headers: { 'Content-Type': 'application/activity+json' })
 
       subject.call(actor)
     end
@@ -76,7 +76,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do
   describe '#call' do
     context 'when the endpoint is a Collection' do
       before do
-        stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload))
+        stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       it_behaves_like 'sets pinned posts'
@@ -93,7 +93,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do
       end
 
       before do
-        stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload))
+        stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       it_behaves_like 'sets pinned posts'
@@ -114,7 +114,7 @@ RSpec.describe ActivityPub::FetchFeaturedCollectionService, type: :service do
       end
 
       before do
-        stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload))
+        stub_request(:get, actor.featured_collection_url).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       it_behaves_like 'sets pinned posts'
diff --git a/spec/services/activitypub/fetch_remote_account_service_spec.rb b/spec/services/activitypub/fetch_remote_account_service_spec.rb
index c05e022aa2..1a649c9540 100644
--- a/spec/services/activitypub/fetch_remote_account_service_spec.rb
+++ b/spec/services/activitypub/fetch_remote_account_service_spec.rb
@@ -42,7 +42,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
       before do
         actor[:inbox] = nil
 
-        stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor))
+        stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' })
         stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })
       end
 
@@ -65,7 +65,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
       let!(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
 
       before do
-        stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor))
+        stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' })
         stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })
       end
 
@@ -91,7 +91,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
       let!(:webfinger) { { subject: 'acct:alice@iscool.af', links: [{ rel: 'self', href: 'https://example.com/alice' }] } }
 
       before do
-        stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor))
+        stub_request(:get, 'https://example.com/alice').to_return(body: Oj.dump(actor), headers: { 'Content-Type': 'application/activity+json' })
         stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:alice@example.com').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })
         stub_request(:get, 'https://iscool.af/.well-known/webfinger?resource=acct:alice@iscool.af').to_return(body: Oj.dump(webfinger), headers: { 'Content-Type': 'application/jrd+json' })
       end
diff --git a/spec/services/activitypub/fetch_replies_service_spec.rb b/spec/services/activitypub/fetch_replies_service_spec.rb
index fe49b18c19..ec40abd5b2 100644
--- a/spec/services/activitypub/fetch_replies_service_spec.rb
+++ b/spec/services/activitypub/fetch_replies_service_spec.rb
@@ -41,7 +41,7 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
 
       context 'when passing the URL to the collection' do
         before do
-          stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
+          stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
         end
 
         it 'spawns workers for up to 5 replies on the same server' do
@@ -70,7 +70,7 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
 
       context 'when passing the URL to the collection' do
         before do
-          stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
+          stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
         end
 
         it 'spawns workers for up to 5 replies on the same server' do
@@ -103,7 +103,7 @@ RSpec.describe ActivityPub::FetchRepliesService, type: :service do
 
       context 'when passing the URL to the collection' do
         before do
-          stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
+          stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
         end
 
         it 'spawns workers for up to 5 replies on the same server' do
diff --git a/spec/services/activitypub/synchronize_followers_service_spec.rb b/spec/services/activitypub/synchronize_followers_service_spec.rb
index 75dcf204b7..7b4a5f8ffe 100644
--- a/spec/services/activitypub/synchronize_followers_service_spec.rb
+++ b/spec/services/activitypub/synchronize_followers_service_spec.rb
@@ -58,7 +58,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do
   describe '#call' do
     context 'when the endpoint is a Collection of actor URIs' do
       before do
-        stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
+        stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       it_behaves_like 'synchronizes followers'
@@ -75,7 +75,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do
       end
 
       before do
-        stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
+        stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       it_behaves_like 'synchronizes followers'
@@ -96,7 +96,7 @@ RSpec.describe ActivityPub::SynchronizeFollowersService, type: :service do
       end
 
       before do
-        stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
+        stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload), headers: { 'Content-Type': 'application/activity+json' })
       end
 
       it_behaves_like 'synchronizes followers'
diff --git a/spec/workers/activitypub/fetch_replies_worker_spec.rb b/spec/workers/activitypub/fetch_replies_worker_spec.rb
index 91ef3c4b92..64cfcd8cbf 100644
--- a/spec/workers/activitypub/fetch_replies_worker_spec.rb
+++ b/spec/workers/activitypub/fetch_replies_worker_spec.rb
@@ -21,7 +21,7 @@ describe ActivityPub::FetchRepliesWorker do
 
   describe 'perform' do
     it 'performs a request if the collection URI is from the same host' do
-      stub_request(:get, 'https://example.com/statuses_replies/1').to_return(status: 200, body: json)
+      stub_request(:get, 'https://example.com/statuses_replies/1').to_return(status: 200, body: json, headers: { 'Content-Type': 'application/activity+json' })
       subject.perform(status.id, 'https://example.com/statuses_replies/1')
       expect(a_request(:get, 'https://example.com/statuses_replies/1')).to have_been_made.once
     end