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 85a91c8b81
commit 2f8ab8cd30
5 changed files with 1694 additions and 11 deletions

View file

@ -1,6 +1,7 @@
import computed from 'ember-addons/ember-computed-decorators';
import DiscourseURL from 'discourse/lib/url';
import { emojiUnescape } from 'discourse/lib/text';
import { escapeExpression } from 'discourse/lib/utilities';
export default Ember.Component.extend({
size: 'medium',
@ -39,10 +40,10 @@ export default Ember.Component.extend({
if (size === 'large') {
const longDescription = this.get('badge.long_description');
if (!_.isEmpty(longDescription)) {
return emojiUnescape(longDescription);
return emojiUnescape(escapeExpression(longDescription));
}
}
return this.get('badge.description');
return escapeExpression(this.get('badge.description'));
}
});

View file

@ -291,7 +291,8 @@ class SessionController < ApplicationController
message = user.suspend_reason ? "login.suspended_with_reason" : "login.suspended"
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'
}
end

View file

@ -407,10 +407,16 @@ describe SessionController do
describe 'suspended user' do
it 'should return an error' do
User.any_instance.stubs(:suspended?).returns(true)
User.any_instance.stubs(:suspended_till).returns(2.days.from_now)
user.suspended_till = 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'
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

View file

@ -12,5 +12,6 @@ test("Visit Badge Pages", () => {
andThen(() => {
ok(exists('.badge-card'), "has the badge in the listing");
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