From 2af4f3c4e22ab9a28a7fca49bee0ee2ed6256f27 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 27 Apr 2017 17:06:47 +0200 Subject: Improve shared status verification (#2525) * Instead of parsing shared status contents verbatim, make roundtrip to purported original URL. Confirm that the "original" URL is from the same domain as the author it claims to be from. * Fix obvious typo, add comment * Use URI look-up first * Add test, update Goldfinger dependency to make less useless HTTP requests per Webfinger lookup --- .../services/follow_remote_account_service_spec.rb | 1 + spec/services/process_feed_service_spec.rb | 138 +++++++++++++++------ 2 files changed, 103 insertions(+), 36 deletions(-) (limited to 'spec') diff --git a/spec/services/follow_remote_account_service_spec.rb b/spec/services/follow_remote_account_service_spec.rb index a8d4a7c6b..66e9eb6c9 100644 --- a/spec/services/follow_remote_account_service_spec.rb +++ b/spec/services/follow_remote_account_service_spec.rb @@ -5,6 +5,7 @@ RSpec.describe FollowRemoteAccountService do before do stub_request(:get, "https://quitter.no/.well-known/host-meta").to_return(request_fixture('.host-meta.txt')) + stub_request(:get, "https://example.com/.well-known/webfinger?resource=acct:catsrgr8@example.com").to_return(status: 404) stub_request(:get, "https://example.com/.well-known/host-meta").to_return(status: 404) stub_request(:get, "https://quitter.no/.well-known/webfinger?resource=acct:gargron@quitter.no").to_return(request_fixture('webfinger.txt')) stub_request(:get, "https://quitter.no/.well-known/webfinger?resource=acct:catsrgr8@quitter.no").to_return(status: 404) diff --git a/spec/services/process_feed_service_spec.rb b/spec/services/process_feed_service_spec.rb index b15284fee..7f2b4ce1d 100644 --- a/spec/services/process_feed_service_spec.rb +++ b/spec/services/process_feed_service_spec.rb @@ -1,52 +1,118 @@ require 'rails_helper' RSpec.describe ProcessFeedService do - let(:body) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'xml', 'mastodon.atom')) } - let(:account) { Fabricate(:account, username: 'localhost', domain: 'kickass.zone') } - subject { ProcessFeedService.new } - before do - stub_request(:post, "https://pubsubhubbub.superfeedr.com/").to_return(:status => 200, :body => "", :headers => {}) - stub_request(:get, "http://kickass.zone/system/accounts/avatars/000/000/001/large/eris.png").to_return(request_fixture('avatar.txt')) - stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/002/original/morpheus_linux.jpg?1476059910").to_return(request_fixture('attachment1.txt')) - stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/003/original/gizmo.jpg?1476060065").to_return(request_fixture('attachment2.txt')) + describe 'processing a feed' do + let(:body) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'xml', 'mastodon.atom')) } + let(:account) { Fabricate(:account, username: 'localhost', domain: 'kickass.zone') } - subject.call(body, account) - end + before do + stub_request(:post, "https://pubsubhubbub.superfeedr.com/").to_return(:status => 200, :body => "", :headers => {}) + stub_request(:get, "http://kickass.zone/system/accounts/avatars/000/000/001/large/eris.png").to_return(request_fixture('avatar.txt')) + stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/002/original/morpheus_linux.jpg?1476059910").to_return(request_fixture('attachment1.txt')) + stub_request(:get, "http://kickass.zone/system/media_attachments/files/000/000/003/original/gizmo.jpg?1476060065").to_return(request_fixture('attachment2.txt')) - it 'updates remote user\'s account information' do - account.reload - expect(account.display_name).to eq '::1' - expect(account).to have_attached_file(:avatar) - end + subject.call(body, account) + end - it 'creates posts' do - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=1:objectType=Status')).to_not be_nil - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Status')).to_not be_nil - end + it 'updates remote user\'s account information' do + account.reload + expect(account.display_name).to eq '::1' + expect(account).to have_attached_file(:avatar) + end - it 'ignores delete statuses unless they existed before' do - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=3:objectType=Status')).to be_nil - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=12:objectType=Status')).to be_nil - end + it 'creates posts' do + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=1:objectType=Status')).to_not be_nil + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Status')).to_not be_nil + end - it 'does not create statuses for follows' do - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=1:objectType=Follow')).to be_nil - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Follow')).to be_nil - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=4:objectType=Follow')).to be_nil - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=7:objectType=Follow')).to be_nil - end + it 'ignores delete statuses unless they existed before' do + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=3:objectType=Status')).to be_nil + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=12:objectType=Status')).to be_nil + end + + it 'does not create statuses for follows' do + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=1:objectType=Follow')).to be_nil + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Follow')).to be_nil + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=4:objectType=Follow')).to be_nil + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=7:objectType=Follow')).to be_nil + end - it 'does not create statuses for favourites' do - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Favourite')).to be_nil - expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=3:objectType=Favourite')).to be_nil + it 'does not create statuses for favourites' do + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=2:objectType=Favourite')).to be_nil + expect(Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=3:objectType=Favourite')).to be_nil + end + + it 'creates posts with media' do + status = Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=14:objectType=Status') + + expect(status).to_not be_nil + expect(status.media_attachments.first).to have_attached_file(:file) + end end - it 'creates posts with media' do - status = Status.find_by(uri: 'tag:kickass.zone,2016-10-10:objectId=14:objectType=Status') + it 'does not accept tampered reblogs' do + good_actor = Fabricate(:account, username: 'tracer', domain: 'overwatch.com') + + real_body = < + + tag:overwatch.com,2017-04-27:objectId=4467137:objectType=Status + 2017-04-27T13:49:25Z + 2017-04-27T13:49:25Z + http://activitystrea.ms/schema/1.0/note + http://activitystrea.ms/schema/1.0/post + + https://overwatch.com/users/tracer + http://activitystrea.ms/schema/1.0/person + https://overwatch.com/users/tracer + tracer + + Overwatch rocks + +XML + + stub_request(:head, 'https://overwatch.com/users/tracer/updates/1').to_return(status: 200, headers: { 'Content-Type' => 'application/atom+xml' }) + stub_request(:get, 'https://overwatch.com/users/tracer/updates/1').to_return(status: 200, body: real_body) + + bad_actor = Fabricate(:account, username: 'sombra', domain: 'talon.xyz') + + body = < + + tag:talon.xyz,2017-04-27:objectId=4467137:objectType=Status + 2017-04-27T13:49:25Z + 2017-04-27T13:49:25Z + + https://talon.xyz/users/sombra + http://activitystrea.ms/schema/1.0/person + https://talon.xyz/users/sombra + sombra + + http://activitystrea.ms/schema/1.0/activity + http://activitystrea.ms/schema/1.0/share + Overwatch SUCKS AHAHA + + tag:overwatch.com,2017-04-27:objectId=4467137:objectType=Status + http://activitystrea.ms/schema/1.0/note + http://activitystrea.ms/schema/1.0/post + + https://overwatch.com/users/tracer + http://activitystrea.ms/schema/1.0/person + https://overwatch.com/users/tracer + tracer + + Overwatch SUCKS AHAHA + + + +XML + created_statuses = subject.call(body, bad_actor) - expect(status).to_not be_nil - expect(status.media_attachments.first).to have_attached_file(:file) + expect(created_statuses.first.reblog?).to be true + expect(created_statuses.first.account_id).to eq bad_actor.id + expect(created_statuses.first.reblog.account_id).to eq good_actor.id + expect(created_statuses.first.reblog.text).to eq 'Overwatch rocks' end end -- cgit