From 5d549f553b488c7e71f11f6a5ae4d15ec1b37fa3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonne=20Ha=C3=9F?= <me@mrzyx.de>
Date: Sat, 30 Aug 2014 20:04:36 +0200
Subject: [PATCH] Escape person name in contacts json

jQuery autoSuggest uses .html to insert it into the DOM
---
 Changelog.md                                  |  1 +
 app/controllers/conversations_controller.rb   |  2 +-
 .../conversations_controller_spec.rb          | 28 ++++++++++++-------
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/Changelog.md b/Changelog.md
index 75221df57..5cd2718e4 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -22,6 +22,7 @@
 * 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 180c42bf6..8322cfb3c 100644
--- a/app/controllers/conversations_controller.rb
+++ b/app/controllers/conversations_controller.rb
@@ -85,7 +85,7 @@ class ConversationsController < ApplicationController
     all_contacts_and_ids = Contact.connection.select_rows(
       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 3c220c129..d75073bb6 100644
--- a/spec/controllers/conversations_controller_spec.rb
+++ b/spec/controllers/conversations_controller_spec.rb
@@ -10,15 +10,13 @@ describe ConversationsController do
   end
 
   describe '#new' do
-    before do
-      get :new
-    end
-
     it 'succeeds' do
+      get :new
       response.should be_success
     end
 
     it "assigns a json list of contacts that are sharing with the person" do
+      get :new
       assigns(:contacts_json).should 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)
       assigns(:contacts_json).should_not include(alice.contacts.where(:sharing => false).first.person.name)
@@ -41,6 +39,16 @@ describe ConversationsController do
         response.body.should_not include xss
       end
     end
+
+    it "does not allow XSS via the profile name" do
+      xss = "<script>alert(0);</script>"
+      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
@@ -53,20 +61,20 @@ describe ConversationsController do
       }
       @conversations = Array.new(3) { Conversation.create(hash) }
     end
-    
+
     it 'succeeds' do
       get :index
       response.should be_success
       assigns[:conversations].should =~ @conversations
     end
-    
+
     it 'succeeds with json' do
       get :index, :format => :json
       response.should be_success
       json = JSON.parse(response.body)
       json.first['conversation'].should be_present
     end
-    
+
     it 'retrieves all conversations for a user' do
       get :index
       assigns[:conversations].count.should == 3
@@ -254,13 +262,13 @@ describe ConversationsController do
       }
       @conversation = Conversation.create(hash)
     end
-    
+
     it 'succeeds with js' do
       get :show, :id => @conversation.id, :format => :js
       response.should be_success
       assigns[:conversation].should == @conversation
     end
-    
+
     it 'succeeds with json' do
       get :show, :id => @conversation.id, :format => :json
       response.should be_success
@@ -273,7 +281,7 @@ describe ConversationsController do
       response.should redirect_to(conversations_path(:conversation_id => @conversation.id))
       assigns[:conversation].should == @conversation
     end
-    
+
     it 'does not let you access conversations where you are not a recipient' do
       sign_in :user, eve