From e6b1111fd13aa7871b8903d016c03488b9ece0cd Mon Sep 17 00:00:00 2001 From: Maxwell Salzberg Date: Sat, 3 Mar 2012 17:50:51 -0800 Subject: [PATCH] AHHHH refactor attack_vectors_spec; hopefully it now borders on understandiablity. this deffy needs another pass from another human, as I had to make tons of spec helper methods to make the crazy setup make sense, but I think we are bordering on something that would let us write new tests in a sane way. whether all of these tests now make sense to test in the first place, that is another question all togther, as it was outside the scope of this refactor --- spec/integration/attack_vectors_spec.rb | 334 +++++++++++++----------- 1 file changed, 176 insertions(+), 158 deletions(-) diff --git a/spec/integration/attack_vectors_spec.rb b/spec/integration/attack_vectors_spec.rb index 3c72fba0a..9e2f3ac4f 100644 --- a/spec/integration/attack_vectors_spec.rb +++ b/spec/integration/attack_vectors_spec.rb @@ -4,220 +4,238 @@ require 'spec_helper' + +def receive(post, opts) + sender = opts.fetch(:from) + receiver = opts.fetch(:by) + salmon_xml = sender.salmon(post).xml_for(receiver.person) + zord = Postzord::Receiver::Private.new(receiver, :salmon_xml => salmon_xml) + zord.perform! +end + +def temporary_user(&block) + user = Factory(:user) + block_return_value = yield user + user.delete + block_return_value +end + +def temporary_post(user, &block) + temp_post = user.post(:status_message, :text => 'hi') + block_return_value = yield temp_post + temp_post.delete + block_return_value +end + +def expect_error(partial_message, &block) + begin + yield + rescue => e + e.message.should match partial_message + end +end + +def bogus_retraction(&block) + ret = Retraction.new + yield ret + ret +end + +def user_should_not_see_guid(user, guid) + user.reload.visible_shareables(Post).where(:guid => guid).should be_blank +end + #returns the message +def legit_post_from_user1_to_user2(user1, user2) + original_message = user1.post(:status_message, :text => 'store this!', :to => user1.aspects.find_by_name("generic").id) + salmon_xml = user1.salmon(original_message).xml_for(user2.person) + zord = Postzord::Receiver::Private.new(user2, :salmon_xml => salmon_xml) + zord.perform! + original_message +end + describe "attack vectors" do let(:eves_aspect) { eve.aspects.find_by_name("generic") } let(:alices_aspect) { alice.aspects.find_by_name("generic") } - context 'non-contact valid user' do - it 'does not save a post from a non-contact' do - bad_user = Factory(:user) + context "testing side effects of validation phase" do + describe 'Contact Required Unless Request' do + it 'does not save a post from a non-contact as a side effect' do + salmon_xml = nil + bad_post_guid = nil - post_from_non_contact = bad_user.build_post( :status_message, :text => 'hi') - salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(bob.person) + temporary_user do |bad_user| + temporary_post(bad_user) do |post_from_non_contact| + bad_post_guid = post_from_non_contact.guid + salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(bob.person) + end + end - post_from_non_contact.delete - bad_user.delete - post_count = Post.count + zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - expect { - zord.perform! - }.should raise_error /Contact required unless request/ + expect { + expect_error /Contact required/ do + zord.perform! + end + }.should_not change(Post, :count) - bob.visible_shareables(Post).include?(post_from_non_contact).should be_false - Post.count.should == post_count + user_should_not_see_guid(bob, bad_post_guid) + end + + it 'other users can not grant visiblity to another users posts by sending their friends post to themselves (even if they are contacts)' do + #setup: eve has a message. then, alice is connected to eve. + #(meaning alice can not see the old post, but it exists in the DB) + # bob takes eves message, changes the post author to himself + # bob trys to send a message to alice + original_message = eve.post(:status_message, :text => 'store this!', :to => eves_aspect.id) + original_message.diaspora_handle = bob.diaspora_handle + + alice.contacts.create(:person => eve.person, :aspects => [alice.aspects.first]) + + salmon_xml = bob.salmon(original_message).xml_for(alice.person) + + #bob sends it to himself????? + zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) + + expect_error /Contact required/ do + zord.perform! + end + + #alice still should not see eves original post, even though bob sent it to her + user_should_not_see_guid(alice, original_message.guid) + end + end + + describe 'author does not match xml author' do + it 'should not overwrite another persons profile profile' do + profile = eve.profile.clone + profile.first_name = "Not BOB" + + expect { + expect_error /Author does not match XML author/ do + receive(profile, :from => alice, :by => bob) + end + }.should_not change(eve.profile, :first_name) + end end end - it 'does not let a user attach to posts previously in the db unless its received from the author' do - original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id - original_message.diaspora_handle = bob.diaspora_handle - alice.contacts.create(:person => eve.person, :aspects => [alice.aspects.first]) - - salmon_xml = bob.salmon(original_message).xml_for(alice.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - expect { - zord.perform! - }.should raise_error /Contact required unless request/ - - alice.reload.visible_shareables(Post).should_not include(StatusMessage.find(original_message.id)) - end context 'malicious contact attack vector' do describe 'mass assignment on id' do it "does not save a message over an old message with a different author" do - original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id + #setup: A user has a message with a given guid and author + original_message = legit_post_from_user1_to_user2(eve, bob) - salmon_xml = eve.salmon(original_message).xml_for(bob.person) + #someone else tries to make a message with the same guid + malicious_message = Factory.build(:status_message, :id => original_message.id, :guid => original_message.guid, :author => alice.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! - - malicious_message = Factory.build(:status_message, :id => original_message.id, :text => 'BAD!!!', :author => alice.person) - salmon_xml = alice.salmon(malicious_message).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! - - original_message.reload.text.should == "store this!" + expect{ + receive(malicious_message, :from => alice, :by => bob) + }.should_not change(original_message, :author_id) end it 'does not save a message over an old message with the same author' do - original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id + #setup: + # i have a legit message from eve + original_message = legit_post_from_user1_to_user2(eve, bob) - salmon_xml = eve.salmon(original_message).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! + #eve tries to send me another message with the same ID + malicious_message = Factory.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => eve.person) - lambda { - malicious_message = Factory.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => eve.person) - - salmon_xml2 = alice.salmon(malicious_message).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! - - }.should_not change{ - bob.reload.visible_shareables(Post).count - } - - original_message.reload.text.should == "store this!" - bob.visible_shareables(Post).first.text.should == "store this!" + expect { + receive(malicious_message, :from => eve, :by => bob) + }.should_not change(original_message, :text) end end - it 'should not overwrite another persons profile profile' do - profile = eve.profile.clone - profile.first_name = "Not BOB" - - eve.reload - - first_name = eve.profile.first_name - salmon_xml = alice.salmon(profile).xml_for(bob.person) - - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - expect { - zord.perform! - }.should raise_error /Author does not match XML author/ - - eve.reload.profile.first_name.should == first_name - end it "ignores retractions on a post not owned by the retraction's sender" do - StatusMessage.delete_all - original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id + original_message = legit_post_from_user1_to_user2(eve, bob) - salmon_xml = eve.salmon(original_message).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! + ret = bogus_retraction do |retraction| + retraction.post_guid = original_message.guid + retraction.diaspora_handle = alice.person.diaspora_handle + retraction.type = original_message.class.to_s + end - bob.visible_shareables(Post).count.should == 1 - StatusMessage.count.should == 1 - - ret = Retraction.new - ret.post_guid = original_message.guid - ret.diaspora_handle = alice.person.diaspora_handle - ret.type = original_message.class.to_s - - salmon_xml = alice.salmon(ret).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! - - StatusMessage.count.should == 1 - bob.visible_shareables(Post).count.should == 1 + expect { + receive(ret, :from => alice, :by => bob) + }.should_not change(StatusMessage, :count) end - it "disregards retractions for non-existent posts that are from someone other than the post's author" do - StatusMessage.delete_all - original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id - id = original_message.reload.id - - ret = Retraction.new - ret.post_guid = original_message.guid - ret.diaspora_handle = alice.person.diaspora_handle - ret.type = original_message.class.to_s - - original_message.delete - - StatusMessage.count.should == 0 - proc { - salmon_xml = alice.salmon(ret).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! + it "silently disregards retractions for non-existent posts(that are from someone other than the post's author)" do + bogus_retraction = temporary_post(eve) do |original_message| + bogus_retraction do |ret| + ret.post_guid = original_message.guid + ret.diaspora_handle = alice.person.diaspora_handle + ret.type = original_message.class.to_s + end + end + expect{ + receive(bogus_retraction, :from => alice, :by => bob) }.should_not raise_error end it 'should not receive retractions where the retractor and the salmon author do not match' do - original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id + original_message = legit_post_from_user1_to_user2(eve, bob) - salmon_xml = eve.salmon(original_message).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! + retraction = bogus_retraction do |ret| + ret.post_guid = original_message.guid + ret.diaspora_handle = eve.person.diaspora_handle + ret.type = original_message.class.to_s + end - bob.visible_shareables(Post).count.should == 1 - - ret = Retraction.new - ret.post_guid = original_message.guid - ret.diaspora_handle = eve.person.diaspora_handle - ret.type = original_message.class.to_s - - salmon_xml = alice.salmon(ret).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) expect { - zord.perform! - }.should raise_error /Author does not match XML author/ + expect_error /Author does not match XML author/ do + receive(retraction, :from => alice, :by => bob) + end + }.should_not change(bob.visible_shareables(Post), :count) - bob.reload.visible_shareables(Post).count.should == 1 end it 'it should not allow you to send retractions for other people' do - ret = Retraction.new - ret.post_guid = eve.person.guid - ret.diaspora_handle = alice.person.diaspora_handle - ret.type = eve.person.class.to_s + #we are banking on bob being friends with alice and eve + #here, alice is trying to disconnect bob and eve - proc{ - salmon_xml = alice.salmon(ret).xml_for(bob.person) - - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! + retraction = bogus_retraction do |ret| + ret.post_guid = eve.person.guid + ret.diaspora_handle = alice.person.diaspora_handle + ret.type = eve.person.class.to_s + end + expect{ + receive(retraction, :from => alice, :by => bob) }.should_not change{bob.reload.contacts.count} end it 'it should not allow you to send retractions with xml and salmon handle mismatch' do - ret = Retraction.new - ret.post_guid = eve.person.guid - ret.diaspora_handle = eve.person.diaspora_handle - ret.type = eve.person.class.to_s + retraction = bogus_retraction do |ret| + ret.post_guid = eve.person.guid + ret.diaspora_handle = eve.person.diaspora_handle + ret.type = eve.person.class.to_s + end - bob.contacts.count.should == 2 - - salmon_xml = alice.salmon(ret).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - expect { - zord.perform! - }.should raise_error /Author does not match XML author/ - - bob.reload.contacts.count.should == 2 + expect{ + expect_error /Author does not match XML author/ do + receive(retraction, :from => alice, :by => bob) + end + }.should_not change(bob.contacts, :count) end - it 'does not let me update other persons post' do + it 'does not let another user update other persons post' do original_message = eve.post(:photo, :user_file => uploaded_photo, :text => "store this!", :to => eves_aspect.id) + receive(original_message, :from => eve, :by => bob) - salmon_xml = eve.salmon(original_message).xml_for(bob.person) - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! + #is this testing two things? + new_message = original_message.dup + new_message.diaspora_handle = alice.diaspora_handle + new_message.text = "bad bad bad" - original_message.diaspora_handle = alice.diaspora_handle - original_message.text= "bad bad bad" - - salmon_xml = alice.salmon(original_message).xml_for(bob.person) - - zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml) - zord.perform! - - original_message.reload.text.should == "store this!" + expect{ + receive(new_message, :from => alice, :by => bob) + }.should_not change(original_message, :text) end end end