Fix bad URL schemes being accepted (#6219)

* Fix actors accepting invalid URI schemes or different host between URI and URL

* Fix statuses accepting invalid URI scheme or different host to actor

* Adjust tests to new requirements

* Improve readability of mismatching_origin?/invalid_origin? methods
This commit is contained in:
Eugen Rochko 2018-01-08 05:00:23 +01:00 committed by GitHub
parent 93555182c3
commit e4a241abef
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 62 additions and 24 deletions

View file

@ -39,6 +39,10 @@ module JsonLdHelper
!json.nil? && equals_or_includes?(json['@context'], ActivityPub::TagManager::CONTEXT) !json.nil? && equals_or_includes?(json['@context'], ActivityPub::TagManager::CONTEXT)
end end
def unsupported_uri_scheme?(uri)
!uri.start_with?('http://', 'https://')
end
def canonicalize(json) def canonicalize(json)
graph = RDF::Graph.new << JSON::LD::API.toRdf(json) graph = RDF::Graph.new << JSON::LD::API.toRdf(json)
graph.dump(:normalize) graph.dump(:normalize)

View file

@ -5,7 +5,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
CONVERTED_TYPES = %w(Image Video Article).freeze CONVERTED_TYPES = %w(Image Video Article).freeze
def perform def perform
return if delete_arrived_first?(object_uri) || unsupported_object_type? return if delete_arrived_first?(object_uri) || unsupported_object_type? || invalid_origin?(@object['id'])
RedisLock.acquire(lock_options) do |lock| RedisLock.acquire(lock_options) do |lock|
if lock.acquired? if lock.acquired?
@ -213,7 +213,14 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
def object_url def object_url
return if @object['url'].blank? return if @object['url'].blank?
url_to_href(@object['url'], 'text/html')
url_candidate = url_to_href(@object['url'], 'text/html')
if invalid_origin?(url_candidate)
nil
else
url_candidate
end
end end
def content_language_map? def content_language_map?
@ -245,6 +252,15 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
@skip_download ||= DomainBlock.find_by(domain: @account.domain)&.reject_media? @skip_download ||= DomainBlock.find_by(domain: @account.domain)&.reject_media?
end end
def invalid_origin?(url)
return true if unsupported_uri_scheme?(url)
needle = Addressable::URI.parse(url).host
haystack = Addressable::URI.parse(@account.uri).host
!haystack.casecmp(needle).zero?
end
def reply_to_local? def reply_to_local?
!replied_to_status.nil? && replied_to_status.account.local? !replied_to_status.nil? && replied_to_status.account.local?
end end

View file

@ -6,7 +6,7 @@ class ActivityPub::ProcessAccountService < BaseService
# Should be called with confirmed valid JSON # Should be called with confirmed valid JSON
# and WebFinger-resolved username and domain # and WebFinger-resolved username and domain
def call(username, domain, json) def call(username, domain, json)
return if json['inbox'].blank? return if json['inbox'].blank? || unsupported_uri_scheme?(json['id'])
@json = json @json = json
@uri = @json['id'] @uri = @json['id']
@ -107,7 +107,21 @@ class ActivityPub::ProcessAccountService < BaseService
def url def url
return if @json['url'].blank? return if @json['url'].blank?
url_to_href(@json['url'], 'text/html')
url_candidate = url_to_href(@json['url'], 'text/html')
if unsupported_uri_scheme?(url_candidate) || mismatching_origin?(url_candidate)
nil
else
url_candidate
end
end
def mismatching_origin?(url)
needle = Addressable::URI.parse(url).host
haystack = Addressable::URI.parse(@uri).host
!haystack.casecmp(needle).zero?
end end
def outbox_total_items def outbox_total_items

View file

@ -6,7 +6,7 @@ RSpec.describe ActivityPub::Activity::Create do
let(:json) do let(:json) do
{ {
'@context': 'https://www.w3.org/ns/activitystreams', '@context': 'https://www.w3.org/ns/activitystreams',
id: 'foo', id: [ActivityPub::TagManager.instance.uri_for(sender), '#foo'].join,
type: 'Create', type: 'Create',
actor: ActivityPub::TagManager.instance.uri_for(sender), actor: ActivityPub::TagManager.instance.uri_for(sender),
object: object_json, object: object_json,
@ -16,6 +16,8 @@ RSpec.describe ActivityPub::Activity::Create do
subject { described_class.new(json, sender) } subject { described_class.new(json, sender) }
before do before do
sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender))
stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt')) stub_request(:get, 'http://example.com/attachment.png').to_return(request_fixture('avatar.txt'))
stub_request(:get, 'http://example.com/emoji.png').to_return(body: attachment_fixture('emojo.png')) stub_request(:get, 'http://example.com/emoji.png').to_return(body: attachment_fixture('emojo.png'))
end end
@ -28,7 +30,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'standalone' do context 'standalone' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
} }
@ -52,7 +54,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'public' do context 'public' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
to: 'https://www.w3.org/ns/activitystreams#Public', to: 'https://www.w3.org/ns/activitystreams#Public',
@ -70,7 +72,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'unlisted' do context 'unlisted' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
cc: 'https://www.w3.org/ns/activitystreams#Public', cc: 'https://www.w3.org/ns/activitystreams#Public',
@ -88,7 +90,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'private' do context 'private' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
to: 'http://example.com/followers', to: 'http://example.com/followers',
@ -108,7 +110,7 @@ RSpec.describe ActivityPub::Activity::Create do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
to: ActivityPub::TagManager.instance.uri_for(recipient), to: ActivityPub::TagManager.instance.uri_for(recipient),
@ -128,7 +130,7 @@ RSpec.describe ActivityPub::Activity::Create do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
inReplyTo: ActivityPub::TagManager.instance.uri_for(original_status), inReplyTo: ActivityPub::TagManager.instance.uri_for(original_status),
@ -151,7 +153,7 @@ RSpec.describe ActivityPub::Activity::Create do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
tag: [ tag: [
@ -174,7 +176,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with mentions missing href' do context 'with mentions missing href' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
tag: [ tag: [
@ -194,7 +196,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with media attachments' do context 'with media attachments' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
attachment: [ attachment: [
@ -218,7 +220,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with media attachments missing url' do context 'with media attachments missing url' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
attachment: [ attachment: [
@ -239,7 +241,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with hashtags' do context 'with hashtags' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
tag: [ tag: [
@ -263,7 +265,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with hashtags missing name' do context 'with hashtags missing name' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum', content: 'Lorem ipsum',
tag: [ tag: [
@ -284,7 +286,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with emojis' do context 'with emojis' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum :tinking:', content: 'Lorem ipsum :tinking:',
tag: [ tag: [
@ -310,7 +312,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with emojis missing name' do context 'with emojis missing name' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum :tinking:', content: 'Lorem ipsum :tinking:',
tag: [ tag: [
@ -333,7 +335,7 @@ RSpec.describe ActivityPub::Activity::Create do
context 'with emojis missing icon' do context 'with emojis missing icon' do
let(:object_json) do let(:object_json) do
{ {
id: 'bar', id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join,
type: 'Note', type: 'Note',
content: 'Lorem ipsum :tinking:', content: 'Lorem ipsum :tinking:',
tag: [ tag: [

View file

@ -21,6 +21,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService do
describe '#call' do describe '#call' do
before do before do
sender.update(uri: ActivityPub::TagManager.instance.uri_for(sender))
stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '') stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '')
subject.call(object[:id], prefetched_body: Oj.dump(object)) subject.call(object[:id], prefetched_body: Oj.dump(object))
end end
@ -48,13 +50,13 @@ RSpec.describe ActivityPub::FetchRemoteStatusService do
{ {
type: 'Link', type: 'Link',
mimeType: 'application/x-bittorrent', mimeType: 'application/x-bittorrent',
href: 'https://example.com/12345.torrent', href: "https://#{valid_domain}/12345.torrent",
}, },
{ {
type: 'Link', type: 'Link',
mimeType: 'text/html', mimeType: 'text/html',
href: 'https://example.com/watch?v=12345', href: "https://#{valid_domain}/watch?v=12345",
}, },
], ],
} }
@ -64,8 +66,8 @@ RSpec.describe ActivityPub::FetchRemoteStatusService do
status = sender.statuses.first status = sender.statuses.first
expect(status).to_not be_nil expect(status).to_not be_nil
expect(status.url).to eq 'https://example.com/watch?v=12345' expect(status.url).to eq "https://#{valid_domain}/watch?v=12345"
expect(strip_tags(status.text)).to eq 'Nyan Cat 10 hours remix https://example.com/watch?v=12345' expect(strip_tags(status.text)).to eq "Nyan Cat 10 hours remix https://#{valid_domain}/watch?v=12345"
end end
end end
end end