diff --git a/app/models/debate.rb b/app/models/debate.rb index 0c98d9274..d82cfb8ac 100644 --- a/app/models/debate.rb +++ b/app/models/debate.rb @@ -13,6 +13,7 @@ class Debate < ActiveRecord::Base validates :terms_of_service, acceptance: { allow_nil: false }, on: :create before_validation :sanitize_description + before_validation :sanitize_tag_list def likes get_likes.size @@ -40,4 +41,8 @@ class Debate < ActiveRecord::Base self.description = WYSIWYGSanitizer.new.sanitize(description) end + def sanitize_tag_list + self.tag_list = TagSanitizer.new.sanitize_tag_list(self.tag_list) + end + end diff --git a/lib/tag_sanitizer.rb b/lib/tag_sanitizer.rb new file mode 100644 index 000000000..f44b06adf --- /dev/null +++ b/lib/tag_sanitizer.rb @@ -0,0 +1,17 @@ +class TagSanitizer + + DISALLOWED_STRINGS = %w(? < > = /) + + def sanitize_tag(tag) + tag = tag.dup + DISALLOWED_STRINGS.each do |s| + tag.gsub!(s, '') + end + tag + end + + def sanitize_tag_list(tag_list) + tag_list.map { |tag| sanitize_tag(tag) } + end + +end diff --git a/spec/features/debates_spec.rb b/spec/features/debates_spec.rb index cbeeee600..034b1d504 100644 --- a/spec/features/debates_spec.rb +++ b/spec/features/debates_spec.rb @@ -62,6 +62,27 @@ feature 'Debates' do expect(page.html).to_not include '' end + scenario 'tagging using dangerous strings' do + + author = create(:user) + login_as(author) + + visit new_debate_path + + fill_in 'debate_title', with: 'A test' + fill_in 'debate_description', with: 'A test' + fill_in 'debate_tag_list', with: 'user_id=1, &a=3, ' + check 'debate_terms_of_service' + + click_button 'Create Debate' + + expect(page).to have_content 'Debate was successfully created.' + expect(page).to have_content 'user_id1' + expect(page).to have_content 'a3' + expect(page).to have_content 'scriptalert("hey");script' + expect(page.html).to_not include 'user_id=1, &a=3, ' + end + scenario 'Update should not be posible if logged user is not the author' do debate = create(:debate) expect(debate).to be_editable diff --git a/spec/lib/tag_sanitizer_spec.rb b/spec/lib/tag_sanitizer_spec.rb new file mode 100644 index 000000000..e1fd6499b --- /dev/null +++ b/spec/lib/tag_sanitizer_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +describe TagSanitizer do + + subject { described_class.new } + + describe '#sanitize_tag' do + it 'allows regular text, even spaces' do + expect(subject.sanitize_tag('hello there')).to eq('hello there') + end + + it 'filters out dangerous strings' do + expect(subject.sanitize_tag('user_id=1')).to eq('user_id1') + end + end + + describe '#sanitize_tag_list' do + it 'returns a new tag list with sanitized tags' do + expect(subject.sanitize_tag_list(%w{x=1 y?z})).to eq(%w(x1 yz)) + end + end + +end diff --git a/spec/models/debate_spec.rb b/spec/models/debate_spec.rb index 2510c7ec1..f2af2cf0a 100644 --- a/spec/models/debate_spec.rb +++ b/spec/models/debate_spec.rb @@ -31,6 +31,12 @@ describe Debate do expect(@debate.description).to eq("alert('danger');") end + it "should sanitize the tag list" do + @debate.tag_list = "user_id=1" + @debate.valid? + expect(@debate.tag_list).to eq(['user_id1']) + end + it "should not be valid without accepting terms of service" do @debate.terms_of_service = nil expect(@debate).to_not be_valid