diff --git a/Changelog.md b/Changelog.md index 00c13b184f8202905b7019909c050cb1b95bbbe8..d4f9a5727d98207bf441eb35e56753e262ff3f4f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +# 0.0.2.4 + +* Fix XSS vulnerabilities caused by not escaping a users name fields when loading it from JSON. [#3948](https://github.com/diaspora/diaspora/issues/3948) + # 0.0.2.3 * Upgrade to Devise 2.1.3 [Read more](http://blog.plataformatec.com.br/2013/01/security-announcement-devise-v2-2-3-v2-1-3-v2-0-5-and-v1-5-3-released/) diff --git a/app/assets/javascripts/widgets/search.js b/app/assets/javascripts/widgets/search.js index 4648cdf45570554faa7fcced1b760c495dc7aadc..675b6f8b874dd13a1c0f6d5b27a2b79cbc8a5e72 100644 --- a/app/assets/javascripts/widgets/search.js +++ b/app/assets/javascripts/widgets/search.js @@ -36,11 +36,12 @@ }; this.formatResult = function(row) { - return row.name; + return Handlebars.Utils.escapeExpression(row.name); }; this.parse = function(data) { var results = data.map(function(person){ + person['name'] = Handlebars.Utils.escapeExpression(person['name']); return {data : person, value : person['name']} }); diff --git a/app/helpers/layout_helper.rb b/app/helpers/layout_helper.rb index 8e6983ce12627909b9d07d38372e6a26b5827dbc..67b1fd71dd076f8d0c8c2b117e5941d13c8f478e 100644 --- a/app/helpers/layout_helper.rb +++ b/app/helpers/layout_helper.rb @@ -42,7 +42,7 @@ module LayoutHelper user = UserPresenter.new(current_user).to_json content_tag(:script) do <<-JS.html_safe - window.current_user_attributes = #{user} + window.current_user_attributes = #{j user} JS end end diff --git a/app/views/people/show.html.haml b/app/views/people/show.html.haml index 6ee3f982aca85f1a65d1fde08023518a6f4aef58..57e3b9a5d7fde334d97cbc17a101a63d506ac698 100644 --- a/app/views/people/show.html.haml +++ b/app/views/people/show.html.haml @@ -6,7 +6,7 @@ - content_for :head do = javascript_include_tag :people :javascript - Mentions.options.prefillMention = Mentions._contactToMention(#{@person.to_json}); + Mentions.options.prefillMention = Mentions._contactToMention(#{j @person.to_json}); - content_for :page_title do = @person.name diff --git a/config/defaults.yml b/config/defaults.yml index 62165e9fed876e153aa4a67983cb5e08429aa037..08acbc33180a61027f6343df5f22fab331350572 100644 --- a/config/defaults.yml +++ b/config/defaults.yml @@ -4,7 +4,7 @@ defaults: version: - number: "0.0.2.2" + number: "0.0.2.4" release: true # Do not touch unless in a merge conflict on doing a release, master should have a commit setting this to true which is not backported to the develop branch. heroku: false environment: diff --git a/config/initializers/json_escape.rb b/config/initializers/json_escape.rb new file mode 100644 index 0000000000000000000000000000000000000000..a27bf2dd5decef70a714d5e89b1b03215e309da4 --- /dev/null +++ b/config/initializers/json_escape.rb @@ -0,0 +1,11 @@ +# From http://jfire.io/blog/2012/04/30/how-to-securely-bootstrap-json-in-a-rails-view/ +# Review on Rails 4 update, might be built in by then! + +class ActionView::Base + def json_escape(s) + result = s.to_s.gsub('/', '\/') + s.html_safe? ? result.html_safe : result + end + + alias j json_escape +end diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index fb311c9b16ebb6f31640372ac914538cfed2dd68..ad08a7529e0deb556e7d58e7c7f3b8653fb26469 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -201,11 +201,10 @@ describe PeopleController do it 'does not allow xss attacks' do user2 = bob profile = user2.profile - profile.first_name = "<script> alert('xss attack');</script>" - profile.save + profile.update_attribute(:first_name, "</script><script> alert('xss attack');</script>") get :show, :id => user2.person.to_param response.should be_success - response.body.match(profile.first_name).should be_false + response.body.should_not include(profile.first_name) end diff --git a/spec/helpers/layout_helper_spec.rb b/spec/helpers/layout_helper_spec.rb index 0dabf3f2ba556d02ca6fe9c023487755b3dca79f..4b0ef1471e6a2d989173254ceaf310a4212b54ff 100644 --- a/spec/helpers/layout_helper_spec.rb +++ b/spec/helpers/layout_helper_spec.rb @@ -10,6 +10,18 @@ describe LayoutHelper do @user = alice end + describe "#set_current_user_in_javascript" do + it "doesn't allow xss" do + user = FactoryGirl.create :user + profile = user.profile + profile.update_attribute(:first_name, "</script><script>alert(0);</script>"); + stub!(:user_signed_in?).and_return true + stub!(:current_user).and_return user + set_current_user_in_javascript.should_not be_empty + set_current_user_in_javascript.should_not include(profile.first_name) + end + end + describe "#page_title" do before do def current_user @@ -30,4 +42,4 @@ describe LayoutHelper do end end end -end \ No newline at end of file +end diff --git a/spec/javascripts/widgets/search-spec.js b/spec/javascripts/widgets/search-spec.js new file mode 100644 index 0000000000000000000000000000000000000000..0e06516c293da3d1a6f666f3f024d156f2235a48 --- /dev/null +++ b/spec/javascripts/widgets/search-spec.js @@ -0,0 +1,12 @@ +describe("Diaspora.Widgets.Search", function() { + describe("parse", function() { + it("escapes a persons name", function() { + $("#jasmine_content").html('<form action="#" id="searchForm"></form>'); + + var search = Diaspora.BaseWidget.instantiate("Search", $("#jasmine_content > #searchForm")); + var person = {"name": "</script><script>alert('xss');</script"}; + result = search.parse([$.extend({}, person)]); + expect(result[0].data.name).toNotEqual(person.name); + }); + }); +});