Fix XSS vulnerabilities caused by not escaping a users name fields when loading it from JSON. #3948
From a quick look at the for us available databases this was not actually used in the wild.
This commit is contained in:
parent
7f865e739c
commit
7134513b28
9 changed files with 47 additions and 8 deletions
|
|
@ -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
|
# 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/)
|
* 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/)
|
||||||
|
|
|
||||||
|
|
@ -36,11 +36,12 @@
|
||||||
};
|
};
|
||||||
|
|
||||||
this.formatResult = function(row) {
|
this.formatResult = function(row) {
|
||||||
return row.name;
|
return Handlebars.Utils.escapeExpression(row.name);
|
||||||
};
|
};
|
||||||
|
|
||||||
this.parse = function(data) {
|
this.parse = function(data) {
|
||||||
var results = data.map(function(person){
|
var results = data.map(function(person){
|
||||||
|
person['name'] = Handlebars.Utils.escapeExpression(person['name']);
|
||||||
return {data : person, value : person['name']}
|
return {data : person, value : person['name']}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -42,7 +42,7 @@ module LayoutHelper
|
||||||
user = UserPresenter.new(current_user).to_json
|
user = UserPresenter.new(current_user).to_json
|
||||||
content_tag(:script) do
|
content_tag(:script) do
|
||||||
<<-JS.html_safe
|
<<-JS.html_safe
|
||||||
window.current_user_attributes = #{user}
|
window.current_user_attributes = #{j user}
|
||||||
JS
|
JS
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
|
|
@ -6,7 +6,7 @@
|
||||||
- content_for :head do
|
- content_for :head do
|
||||||
= javascript_include_tag :people
|
= javascript_include_tag :people
|
||||||
:javascript
|
:javascript
|
||||||
Mentions.options.prefillMention = Mentions._contactToMention(#{@person.to_json});
|
Mentions.options.prefillMention = Mentions._contactToMention(#{j @person.to_json});
|
||||||
|
|
||||||
- content_for :page_title do
|
- content_for :page_title do
|
||||||
= @person.name
|
= @person.name
|
||||||
|
|
|
||||||
|
|
@ -4,7 +4,7 @@
|
||||||
|
|
||||||
defaults:
|
defaults:
|
||||||
version:
|
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.
|
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
|
heroku: false
|
||||||
environment:
|
environment:
|
||||||
|
|
|
||||||
11
config/initializers/json_escape.rb
Normal file
11
config/initializers/json_escape.rb
Normal file
|
|
@ -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
|
||||||
|
|
@ -201,11 +201,10 @@ describe PeopleController do
|
||||||
it 'does not allow xss attacks' do
|
it 'does not allow xss attacks' do
|
||||||
user2 = bob
|
user2 = bob
|
||||||
profile = user2.profile
|
profile = user2.profile
|
||||||
profile.first_name = "<script> alert('xss attack');</script>"
|
profile.update_attribute(:first_name, "</script><script> alert('xss attack');</script>")
|
||||||
profile.save
|
|
||||||
get :show, :id => user2.person.to_param
|
get :show, :id => user2.person.to_param
|
||||||
response.should be_success
|
response.should be_success
|
||||||
response.body.match(profile.first_name).should be_false
|
response.body.should_not include(profile.first_name)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,18 @@ describe LayoutHelper do
|
||||||
@user = alice
|
@user = alice
|
||||||
end
|
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
|
describe "#page_title" do
|
||||||
before do
|
before do
|
||||||
def current_user
|
def current_user
|
||||||
|
|
@ -30,4 +42,4 @@ describe LayoutHelper do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
||||||
12
spec/javascripts/widgets/search-spec.js
Normal file
12
spec/javascripts/widgets/search-spec.js
Normal file
|
|
@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue