Improve federated ID validation (#8372)

* Fix URI not being sufficiently validated with prefetched JSON

* Add additional id validation to OStatus documents, when possible

6 files changed, 23 insertions, 7 deletions

diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index 9d2b6cf00..532397272 100644
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@@ -73,8 +73,10 @@ module JsonLdHelper
     end
   end
 
-  def body_to_json(body)
-    body.is_a?(String) ? Oj.load(body, mode: :strict) : body
+  def body_to_json(body, compare_id: nil)
+    json = body.is_a?(String) ? Oj.load(body, mode: :strict) : body
+    return if compare_id.present? && json['id'] != compare_id
+    json
   rescue Oj::ParseError
     nil
   end
diff --git a/app/lib/ostatus/activity/creation.rb b/app/lib/ostatus/activity/creation.rb
index d3a303a0c..8f8c70052 100644
--- a/app/lib/ostatus/activity/creation.rb
+++ b/app/lib/ostatus/activity/creation.rb
@@ -7,7 +7,7 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
       return [nil, false]
     end
 
-    return [nil, false] if @account.suspended?
+    return [nil, false] if @account.suspended? || invalid_origin?
 
     RedisLock.acquire(lock_options) do |lock|
       if lock.acquired?
@@ -204,6 +204,15 @@ class OStatus::Activity::Creation < OStatus::Activity::Base
     end
   end
 
+  def invalid_origin?
+    return false unless id.start_with?('http') # Legacy IDs cannot be checked
+
+    needle = Addressable::URI.parse(id).normalized_host
+
+    !(needle.casecmp(@account.domain).zero? ||
+      needle.casecmp(Addressable::URI.parse(@account.remote_url.presence || @account.uri).normalized_host).zero?)
+  end
+
   def lock_options
     { redis: Redis.current, key: "create:#{id}" }
   end
diff --git a/app/services/activitypub/fetch_remote_account_service.rb b/app/services/activitypub/fetch_remote_account_service.rb
index 41fec9170..1ec9ee5dd 100644
--- a/app/services/activitypub/fetch_remote_account_service.rb
+++ b/app/services/activitypub/fetch_remote_account_service.rb
@@ -11,7 +11,7 @@ class ActivityPub::FetchRemoteAccountService < BaseService
     @json = if prefetched_body.nil?
               fetch_resource(uri, id)
             else
-              body_to_json(prefetched_body)
+              body_to_json(prefetched_body, compare_id: id ? uri : nil)
             end
 
     return if !supported_context? || !expected_type? || (break_on_redirect && @json['movedTo'].present?)
diff --git a/app/services/activitypub/fetch_remote_key_service.rb b/app/services/activitypub/fetch_remote_key_service.rb
index 505baccd4..df17d9079 100644
--- a/app/services/activitypub/fetch_remote_key_service.rb
+++ b/app/services/activitypub/fetch_remote_key_service.rb
@@ -17,7 +17,7 @@ class ActivityPub::FetchRemoteKeyService < BaseService
         @json = fetch_resource(uri, id)
       end
     else
-      @json = body_to_json(prefetched_body)
+      @json = body_to_json(prefetched_body, compare_id: id ? uri : nil)
     end
 
     return unless supported_context?(@json) && expected_type?
diff --git a/app/services/activitypub/fetch_remote_status_service.rb b/app/services/activitypub/fetch_remote_status_service.rb
index 2b447abb3..469821032 100644
--- a/app/services/activitypub/fetch_remote_status_service.rb
+++ b/app/services/activitypub/fetch_remote_status_service.rb
@@ -8,7 +8,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
     @json = if prefetched_body.nil?
               fetch_resource(uri, id, on_behalf_of)
             else
-              body_to_json(prefetched_body)
+              body_to_json(prefetched_body, compare_id: id ? uri : nil)
             end
 
     return unless supported_context? && expected_type?
diff --git a/app/services/fetch_remote_account_service.rb b/app/services/fetch_remote_account_service.rb
index a0f031a44..cfc560022 100644
--- a/app/services/fetch_remote_account_service.rb
+++ b/app/services/fetch_remote_account_service.rb
@@ -27,7 +27,7 @@ class FetchRemoteAccountService < BaseService
 
     account = author_from_xml(xml.at_xpath('/xmlns:feed', xmlns: OStatus::TagManager::XMLNS), false)
 
-    UpdateRemoteProfileService.new.call(xml, account) unless account.nil?
+    UpdateRemoteProfileService.new.call(xml, account) if account.present? && trusted_domain?(url, account)
 
     account
   rescue TypeError
@@ -37,4 +37,9 @@ class FetchRemoteAccountService < BaseService
     Rails.logger.debug 'Invalid XML or missing namespace'
     nil
   end
+
+  def trusted_domain?(url, account)
+    domain = Addressable::URI.parse(url).normalized_host
+    domain.casecmp(account.domain).zero? || domain.casecmp(Addressable::URI.parse(account.remote_url.presence || account.uri).normalized_host).zero?
+  end
 end