Merge pull request from GHSA-3fjr-858r-92rw

* Fix insufficient origin validation

* Bump version to v4.2.5
This commit is contained in:
Claire 2024-02-01 15:56:46 +01:00 committed by GitHub
parent 4633bb8ce0
commit a6641f828b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
20 changed files with 47 additions and 47 deletions

View file

@ -2,6 +2,12 @@
All notable changes to this project will be documented in this file. All notable changes to this project will be documented in this file.
## [4.2.5] - 2024-02-01
### Security
- Fix insufficient origin validation (CVE-2024-23832, [GHSA-3fjr-858r-92rw](https://github.com/mastodon/mastodon/security/advisories/GHSA-3fjr-858r-92rw))
## [4.2.4] - 2024-01-24 ## [4.2.4] - 2024-01-24
### Fixed ### Fixed

View file

@ -17,6 +17,4 @@ A "vulnerability in Mastodon" is a vulnerability in the code distributed through
| ------- | ---------------- | | ------- | ---------------- |
| 4.2.x | Yes | | 4.2.x | Yes |
| 4.1.x | Yes | | 4.1.x | Yes |
| 4.0.x | No | | < 4.1 | No |
| 3.5.x | Until 2023-12-31 |
| < 3.5 | No |

View file

@ -266,7 +266,7 @@ module SignatureVerification
stoplight_wrap_request { ResolveAccountService.new.call(key_id.delete_prefix('acct:'), suppress_errors: false) } stoplight_wrap_request { ResolveAccountService.new.call(key_id.delete_prefix('acct:'), suppress_errors: false) }
elsif !ActivityPub::TagManager.instance.local_uri?(key_id) elsif !ActivityPub::TagManager.instance.local_uri?(key_id)
account = ActivityPub::TagManager.instance.uri_to_actor(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 account
end end
rescue Mastodon::PrivateNetworkAddressError => e rescue Mastodon::PrivateNetworkAddressError => e

View file

@ -155,8 +155,8 @@ module JsonLdHelper
end end
end end
def fetch_resource(uri, id, on_behalf_of = nil, request_options: {}) def fetch_resource(uri, id_is_known, on_behalf_of = nil, request_options: {})
unless id unless id_is_known
json = fetch_resource_without_id_validation(uri, on_behalf_of) json = fetch_resource_without_id_validation(uri, on_behalf_of)
return if !json.is_a?(Hash) || unsupported_uri_scheme?(json['id']) return if !json.is_a?(Hash) || unsupported_uri_scheme?(json['id'])

View file

@ -154,7 +154,7 @@ class ActivityPub::Activity
if object_uri.start_with?('http') if object_uri.start_with?('http')
return if ActivityPub::TagManager.instance.local_uri?(object_uri) return if ActivityPub::TagManager.instance.local_uri?(object_uri)
ActivityPub::FetchRemoteStatusService.new.call(object_uri, id: true, on_behalf_of: @account.followers.local.first, request_id: @options[:request_id]) ActivityPub::FetchRemoteStatusService.new.call(object_uri, on_behalf_of: @account.followers.local.first, request_id: @options[:request_id])
elsif @object['url'].present? elsif @object['url'].present?
::FetchRemoteStatusService.new.call(@object['url'], request_id: @options[:request_id]) ::FetchRemoteStatusService.new.call(@object['url'], request_id: @options[:request_id])
end end

View file

@ -19,7 +19,7 @@ class ActivityPub::LinkedDataSignature
return unless type == 'RsaSignature2017' return unless type == 'RsaSignature2017'
creator = ActivityPub::TagManager.instance.uri_to_actor(creator_uri) creator = ActivityPub::TagManager.instance.uri_to_actor(creator_uri)
creator = ActivityPub::FetchRemoteKeyService.new.call(creator_uri, id: false) if creator&.public_key.blank? creator = ActivityPub::FetchRemoteKeyService.new.call(creator_uri) if creator&.public_key.blank?
return if creator.nil? return if creator.nil?

View file

@ -2,7 +2,7 @@
class ActivityPub::FetchRemoteAccountService < ActivityPub::FetchRemoteActorService class ActivityPub::FetchRemoteAccountService < ActivityPub::FetchRemoteActorService
# Does a WebFinger roundtrip on each call, unless `only_key` is true # 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, request_id: nil) def call(uri, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil)
actor = super actor = super
return actor if actor.nil? || actor.is_a?(Account) return actor if actor.nil? || actor.is_a?(Account)

View file

@ -10,15 +10,15 @@ class ActivityPub::FetchRemoteActorService < BaseService
SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze SUPPORTED_TYPES = %w(Application Group Organization Person Service).freeze
# Does a WebFinger roundtrip on each call, unless `only_key` is true # 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, request_id: nil) def call(uri, prefetched_body: nil, break_on_redirect: false, only_key: false, suppress_errors: true, request_id: nil)
return if domain_not_allowed?(uri) return if domain_not_allowed?(uri)
return ActivityPub::TagManager.instance.uri_to_actor(uri) if ActivityPub::TagManager.instance.local_uri?(uri) return ActivityPub::TagManager.instance.uri_to_actor(uri) if ActivityPub::TagManager.instance.local_uri?(uri)
@json = begin @json = begin
if prefetched_body.nil? if prefetched_body.nil?
fetch_resource(uri, id) fetch_resource(uri, true)
else else
body_to_json(prefetched_body, compare_id: id ? uri : nil) body_to_json(prefetched_body, compare_id: uri)
end end
rescue Oj::ParseError rescue Oj::ParseError
raise Error, "Error parsing JSON-LD document #{uri}" raise Error, "Error parsing JSON-LD document #{uri}"

View file

@ -6,23 +6,10 @@ class ActivityPub::FetchRemoteKeyService < BaseService
class Error < StandardError; end class Error < StandardError; end
# Returns actor that owns the key # 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? raise Error, 'No key URI given' if uri.blank?
if prefetched_body.nil? @json = fetch_resource(uri, false)
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
raise Error, "Unable to fetch key JSON at #{uri}" if @json.nil? 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) raise Error, "Unsupported JSON-LD context for document #{uri}" unless supported_context?(@json)

View file

@ -8,14 +8,14 @@ class ActivityPub::FetchRemoteStatusService < BaseService
DISCOVERIES_PER_REQUEST = 1000 DISCOVERIES_PER_REQUEST = 1000
# Should be called when uri has already been checked for locality # Should be called when uri has already been checked for locality
def call(uri, id: true, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil) def call(uri, prefetched_body: nil, on_behalf_of: nil, expected_actor_uri: nil, request_id: nil)
return if domain_not_allowed?(uri) return if domain_not_allowed?(uri)
@request_id = request_id || "#{Time.now.utc.to_i}-status-#{uri}" @request_id = request_id || "#{Time.now.utc.to_i}-status-#{uri}"
@json = if prefetched_body.nil? @json = if prefetched_body.nil?
fetch_resource(uri, id, on_behalf_of) fetch_resource(uri, true, on_behalf_of)
else else
body_to_json(prefetched_body, compare_id: id ? uri : nil) body_to_json(prefetched_body, compare_id: uri)
end end
return unless supported_context? return unless supported_context?
@ -65,7 +65,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
def account_from_uri(uri) def account_from_uri(uri)
actor = ActivityPub::TagManager.instance.uri_to_resource(uri, Account) actor = ActivityPub::TagManager.instance.uri_to_resource(uri, Account)
actor = ActivityPub::FetchRemoteAccountService.new.call(uri, id: true, request_id: @request_id) if actor.nil? || actor.possibly_stale? actor = ActivityPub::FetchRemoteAccountService.new.call(uri, request_id: @request_id) if actor.nil? || actor.possibly_stale?
actor actor
end end

View file

@ -277,7 +277,7 @@ class ActivityPub::ProcessAccountService < BaseService
def moved_account def moved_account
account = ActivityPub::TagManager.instance.uri_to_resource(@json['movedTo'], Account) account = ActivityPub::TagManager.instance.uri_to_resource(@json['movedTo'], Account)
account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], id: true, break_on_redirect: true, request_id: @options[:request_id]) account ||= ActivityPub::FetchRemoteAccountService.new.call(@json['movedTo'], break_on_redirect: true, request_id: @options[:request_id])
account account
end end

View file

@ -48,7 +48,15 @@ class FetchResourceService < BaseService
body = response.body_with_limit body = response.body_with_limit
json = body_to_json(body) 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 elsif !terminal
link_header = response['Link'] && parse_link_header(response) link_header = response['Link'] && parse_link_header(response)

View file

@ -56,7 +56,7 @@ services:
web: web:
build: . build: .
image: ghcr.io/mastodon/mastodon:v4.2.4 image: ghcr.io/mastodon/mastodon:v4.2.5
restart: always restart: always
env_file: .env.production env_file: .env.production
command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000" command: bash -c "rm -f /mastodon/tmp/pids/server.pid; bundle exec rails s -p 3000"
@ -77,7 +77,7 @@ services:
streaming: streaming:
build: . build: .
image: ghcr.io/mastodon/mastodon:v4.2.4 image: ghcr.io/mastodon/mastodon:v4.2.5
restart: always restart: always
env_file: .env.production env_file: .env.production
command: node ./streaming command: node ./streaming
@ -95,7 +95,7 @@ services:
sidekiq: sidekiq:
build: . build: .
image: ghcr.io/mastodon/mastodon:v4.2.4 image: ghcr.io/mastodon/mastodon:v4.2.5
restart: always restart: always
env_file: .env.production env_file: .env.production
command: bundle exec sidekiq command: bundle exec sidekiq

View file

@ -13,7 +13,7 @@ module Mastodon
end end
def patch def patch
4 5
end end
def default_prerelease def default_prerelease

View file

@ -60,7 +60,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
allow(ActivityPub::FetchRemoteKeyService).to receive(:new).and_return(service_stub) allow(ActivityPub::FetchRemoteKeyService).to receive(:new).and_return(service_stub)
allow(service_stub).to receive(:call).with('http://example.com/alice', id: false) do allow(service_stub).to receive(:call).with('http://example.com/alice') do
sender.update!(public_key: old_key) sender.update!(public_key: old_key)
sender sender
end end
@ -68,7 +68,7 @@ RSpec.describe ActivityPub::LinkedDataSignature do
it 'fetches key and returns creator' do it 'fetches key and returns creator' do
expect(subject.verify_actor!).to eq sender expect(subject.verify_actor!).to eq sender
expect(service_stub).to have_received(:call).with('http://example.com/alice', id: false).once expect(service_stub).to have_received(:call).with('http://example.com/alice').once
end end
end end

View file

@ -18,7 +18,7 @@ RSpec.describe ActivityPub::FetchRemoteAccountService, type: :service do
end end
describe '#call' do 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 shared_examples 'sets profile data' do
it 'returns an account' do it 'returns an account' do

View file

@ -18,7 +18,7 @@ RSpec.describe ActivityPub::FetchRemoteActorService, type: :service do
end end
describe '#call' do 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 shared_examples 'sets profile data' do
it 'returns an account' do it 'returns an account' do

View file

@ -55,7 +55,7 @@ RSpec.describe ActivityPub::FetchRemoteKeyService, type: :service do
end end
describe '#call' do 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 context 'when the key is a sub-object from the actor' do
before do before do

View file

@ -57,7 +57,7 @@ RSpec.describe FetchResourceService, type: :service do
let(:json) do let(:json) do
{ {
id: 1, id: 'http://example.com/foo',
'@context': ActivityPub::TagManager::CONTEXT, '@context': ActivityPub::TagManager::CONTEXT,
type: 'Note', type: 'Note',
}.to_json }.to_json
@ -83,27 +83,27 @@ RSpec.describe FetchResourceService, type: :service do
let(:content_type) { 'application/activity+json; charset=utf-8' } let(:content_type) { 'application/activity+json; charset=utf-8' }
let(:body) { json } 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 end
context 'when content type is ld+json with profile' do context 'when content type is ld+json with profile' do
let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' } let(:content_type) { 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"' }
let(:body) { json } 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 end
context 'when link header is present' do context 'when link header is present' do
let(:headers) { { 'Link' => '<http://example.com/foo>; rel="alternate"; type="application/activity+json"' } } let(:headers) { { 'Link' => '<http://example.com/foo>; rel="alternate"; type="application/activity+json"' } }
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
context 'when content type is text/html' do context 'when content type is text/html' do
let(:content_type) { 'text/html' } let(:content_type) { 'text/html' }
let(:body) { '<html><head><link rel="alternate" href="http://example.com/foo" type="application/activity+json"/></head></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 end
end end

View file

@ -139,6 +139,7 @@ describe ResolveURLService, type: :service do
stub_request(:get, url).to_return(status: 302, headers: { 'Location' => status_url }) 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 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, 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 end
it 'returns status by url' do it 'returns status by url' do