diff --git a/app/assets/javascripts/discourse/app/controllers/create-invite.js b/app/assets/javascripts/discourse/app/controllers/create-invite.js index 8494aad52f0..553ffaf42d9 100644 --- a/app/assets/javascripts/discourse/app/controllers/create-invite.js +++ b/app/assets/javascripts/discourse/app/controllers/create-invite.js @@ -11,6 +11,7 @@ import Group from "discourse/models/group"; import Invite from "discourse/models/invite"; import I18n from "I18n"; import { FORMAT } from "select-kit/components/future-date-input-selector"; +import { sanitize } from "discourse/lib/text"; export default Controller.extend( ModalFunctionality, @@ -130,7 +131,7 @@ export default Controller.extend( if (result.warnings) { this.setProperties({ - flashText: result.warnings.join(","), + flashText: sanitize(result.warnings.join(",")), flashClass: "warning", flashLink: !this.editing, }); @@ -139,7 +140,7 @@ export default Controller.extend( this.send("closeModal"); } else { this.setProperties({ - flashText: I18n.t("user.invited.invite.invite_saved"), + flashText: sanitize(I18n.t("user.invited.invite.invite_saved")), flashClass: "success", flashLink: !this.editing, }); @@ -148,7 +149,7 @@ export default Controller.extend( }) .catch((e) => this.setProperties({ - flashText: extractError(e), + flashText: sanitize(extractError(e)), flashClass: "error", flashLink: false, }) diff --git a/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs b/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs index 689a85545ed..5acc3dfa1f0 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs @@ -2,7 +2,7 @@ {{/if}} diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 582d89af32f..2c75d65911f 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -200,7 +200,10 @@ class InvitesController < ApplicationController if new_email if Invite.where.not(id: invite.id).find_by(email: new_email.downcase, invited_by_id: current_user.id)&.redeemable? - return render_json_error(I18n.t("invite.invite_exists", email: new_email), status: 409) + return render_json_error( + I18n.t("invite.invite_exists", email: CGI.escapeHTML(new_email)), + status: 409 + ) end end diff --git a/app/models/invite.rb b/app/models/invite.rb index 52c0a97b425..e64f4c69bd2 100644 --- a/app/models/invite.rb +++ b/app/models/invite.rb @@ -62,12 +62,7 @@ class Invite < ActiveRecord::Base if user && user.id != self.invited_users&.first&.user_id @email_already_exists = true - errors.add(:base, I18n.t( - "invite.user_exists", - email: email, - username: user.username, - base_path: Discourse.base_path - )) + errors.add(:base, user_exists_error_msg(email, user.username)) end end @@ -106,12 +101,7 @@ class Invite < ActiveRecord::Base email = Email.downcase(opts[:email]) if opts[:email].present? if user = find_user_by_email(email) - raise UserExists.new(I18n.t( - "invite.user_exists", - email: email, - username: user.username, - base_path: Discourse.base_path - )) + raise UserExists.new(new.user_exists_error_msg(email, user.username)) end if email.present? @@ -293,9 +283,19 @@ class Invite < ActiveRecord::Base self.domain.downcase! if self.domain !~ Invite::DOMAIN_REGEX - self.errors.add(:base, I18n.t('invite.domain_not_allowed', domain: self.domain)) + self.errors.add(:base, I18n.t('invite.domain_not_allowed')) end end + + def user_exists_error_msg(email, username) + sanitized_email = CGI.escapeHTML(email) + sanitized_username = CGI.escapeHTML(username) + + I18n.t( + "invite.user_exists", + email: sanitized_email, username: sanitized_username, base_path: Discourse.base_path + ) + end end # == Schema Information diff --git a/lib/validators/email_validator.rb b/lib/validators/email_validator.rb index eca8c28e5f5..6ce7bd0ded7 100644 --- a/lib/validators/email_validator.rb +++ b/lib/validators/email_validator.rb @@ -5,7 +5,7 @@ class EmailValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) unless value =~ EmailValidator.email_regex if Invite === record && attribute == :email - record.errors.add(:base, I18n.t(:'invite.invalid_email', email: value)) + record.errors.add(:base, I18n.t(:'invite.invalid_email', email: CGI.escapeHTML(value))) else record.errors.add(attribute, I18n.t(:'user.email.invalid')) end diff --git a/spec/models/invite_spec.rb b/spec/models/invite_spec.rb index dd3e6a66452..994aeeabe51 100644 --- a/spec/models/invite_spec.rb +++ b/spec/models/invite_spec.rb @@ -4,6 +4,8 @@ require 'rails_helper' describe Invite do fab!(:user) { Fabricate(:user) } + let(:xss_email) { "email@test.com" } + let(:escaped_email) { "<b onmouseover=alert('wufff!')>email</b><script>alert('test');</script>@test.com" } context 'validators' do it { is_expected.to validate_presence_of :invited_by_id } @@ -14,10 +16,11 @@ describe Invite do expect(invite).to be_valid end - it 'does not allow invites with invalid emails' do - invite = Fabricate.build(:invite, email: 'John Doe ') + it 'escapes the invalid email before attaching the error message' do + invite = Fabricate.build(:invite, email: xss_email) + expect(invite.valid?).to eq(false) - expect(invite.errors.full_messages).to include(I18n.t('invite.invalid_email', email: invite.email)) + expect(invite.errors.full_messages).to include(I18n.t('invite.invalid_email', email: escaped_email)) end it 'does not allow an invite with the same email as an existing user' do @@ -82,6 +85,20 @@ describe Invite do .to raise_error(Invite::UserExists) end + it 'escapes the email_address when raising an existing user error' do + user.email = xss_email + user.save(validate: false) + + expect { Invite.generate(user, email: user.email) } + .to raise_error( + Invite::UserExists, + I18n.t( + 'invite.user_exists', + email: escaped_email, username: user.username, base_path: Discourse.base_path + ) + ) + end + context 'via email' do it 'can be created and a job is enqueued to email the invite' do invite = Invite.generate(user, email: 'test@example.com')