SECURITY: XSS in "Account Suspended" Messages and Badge Descriptions

This commit is contained in:
Robin Ward 2016-07-28 11:36:48 -04:00
parent f319923753
commit 90a3cc7f18
5 changed files with 1693 additions and 11 deletions

View File

@ -36,10 +36,10 @@ export default Ember.Component.extend({
if (size === 'large') { if (size === 'large') {
const longDescription = this.get('badge.long_description'); const longDescription = this.get('badge.long_description');
if (!_.isEmpty(longDescription)) { if (!_.isEmpty(longDescription)) {
return Discourse.Emoji.unescape(longDescription); return Discourse.Emoji.unescape(Discourse.Utilities.escapeExpression(longDescription));
} }
} }
return this.get('badge.description'); return Discourse.Utilities.escapeExpression(this.get('badge.description'));
} }
}); });

View File

@ -271,7 +271,8 @@ class SessionController < ApplicationController
message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended" message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended"
render json: { render json: {
error: I18n.t(message, { date: I18n.l(user.suspended_till, format: :date_only), reason: user.suspend_reason}), error: I18n.t(message, { date: I18n.l(user.suspended_till, format: :date_only),
reason: Rack::Utils.escape_html(user.suspend_reason) }),
reason: 'suspended' reason: 'suspended'
} }
end end

View File

@ -407,10 +407,16 @@ describe SessionController do
describe 'suspended user' do describe 'suspended user' do
it 'should return an error' do it 'should return an error' do
User.any_instance.stubs(:suspended?).returns(true) user.suspended_till = 2.days.from_now
User.any_instance.stubs(:suspended_till).returns(2.days.from_now) user.suspended_at = Time.now
user.save!
StaffActionLogger.new(user).log_user_suspend(user, "<strike>banned</strike>")
xhr :post, :create, login: user.username, password: 'myawesomepassword' xhr :post, :create, login: user.username, password: 'myawesomepassword'
expect(::JSON.parse(response.body)['error']).to be_present
error = ::JSON.parse(response.body)['error']
expect(error).to be_present
expect(error).to match(/banned/)
expect(error).not_to match(/<strike>/)
end end
end end

View File

@ -12,5 +12,6 @@ test("Visit Badge Pages", () => {
andThen(() => { andThen(() => {
ok(exists('.badge-card'), "has the badge in the listing"); ok(exists('.badge-card'), "has the badge in the listing");
ok(exists('.user-info'), "has the list of users with that badge"); ok(exists('.user-info'), "has the list of users with that badge");
ok(!exists('.badge-card:eq(0) strike'));
}); });
}); });

File diff suppressed because one or more lines are too long