diff --git a/Changelog.md b/Changelog.md index cd218f19b..b85998e2d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -60,6 +60,7 @@ The default for including jQuery from a CDN has changed. If you want to continue * Set mention notification as read when viewing post [#5006](https://github.com/diaspora/diaspora/pull/5006) * Set sharing notification as read when viewing profile [#5009](https://github.com/diaspora/diaspora/pull/5009) * Ensure a consistent border on text input elements [#5069](https://github.com/diaspora/diaspora/pull/5069) +* Escape person name in contacts json returned by Conversations#new ## Features * Port admin pages to bootstrap, polish user search results, allow accounts to be closed from the backend [#5046](https://github.com/diaspora/diaspora/pull/5046) diff --git a/app/controllers/conversations_controller.rb b/app/controllers/conversations_controller.rb index 58638651d..1ebc4d81c 100644 --- a/app/controllers/conversations_controller.rb +++ b/app/controllers/conversations_controller.rb @@ -87,7 +87,7 @@ class ConversationsController < ApplicationController current_user.contacts.where(:sharing => true).joins(:person => :profile). select("contacts.id, profiles.first_name, profiles.last_name, people.diaspora_handle").to_sql } - ).map{|r| {:value => r[0], :name => Person.name_from_attrs(r[1], r[2], r[3]).gsub(/(")/, "'")} } + ).map{|r| {:value => r[0], :name => ERB::Util.h(Person.name_from_attrs(r[1], r[2], r[3]).gsub(/(")/, "'"))} } @contact_ids = "" diff --git a/spec/controllers/conversations_controller_spec.rb b/spec/controllers/conversations_controller_spec.rb index bfc5ebf73..a21aa6d02 100644 --- a/spec/controllers/conversations_controller_spec.rb +++ b/spec/controllers/conversations_controller_spec.rb @@ -10,15 +10,13 @@ describe ConversationsController, :type => :controller do end describe '#new' do - before do - get :new - end - it 'succeeds' do + get :new expect(response).to be_success end it "assigns a json list of contacts that are sharing with the person" do + get :new expect(assigns(:contacts_json)).to include(alice.contacts.where(:sharing => true).first.person.name) alice.contacts << Contact.new(:person_id => eve.person.id, :user_id => alice.id, :sharing => false, :receiving => true) expect(assigns(:contacts_json)).not_to include(alice.contacts.where(:sharing => false).first.person.name) @@ -41,6 +39,16 @@ describe ConversationsController, :type => :controller do expect(response.body).not_to include xss end end + + it "does not allow XSS via the profile name" do + xss = "" + contact = alice.contacts.first + contact.person.profile.update_attribute(:first_name, xss) + get :new + json = JSON.parse(assigns(:contacts_json)).first + expect(json['value']).to eq(contact.id.to_s) + expect(json['name']).to_not include(xss) + end end describe '#index' do