From 3a9907f39239532d0cf8c6ae58e80b8c3da9811e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 11 May 2015 20:09:17 +0200 Subject: [PATCH] FIX: prevent self-xss in poll attributes --- plugins/poll/assets/javascripts/poll_dialect.js | 10 ++-------- plugins/poll/plugin.rb | 8 ++++---- plugins/poll/spec/controllers/posts_controller_spec.rb | 8 ++++++++ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/plugins/poll/assets/javascripts/poll_dialect.js b/plugins/poll/assets/javascripts/poll_dialect.js index d6eb5bf7185..126f483314c 100644 --- a/plugins/poll/assets/javascripts/poll_dialect.js +++ b/plugins/poll/assets/javascripts/poll_dialect.js @@ -8,7 +8,7 @@ const WHITELISTED_ATTRIBUTES = ["type", "name", "min", "max", "step", "order", "color", "background", "status"]; const WHITELISTED_STYLES = ["color", "background"]; - const ATTRIBUTES_REGEX = new RegExp("(" + WHITELISTED_ATTRIBUTES.join("|") + ")=['\"]?[^\\s\\]]+['\"]?", "g"); + const ATTRIBUTES_REGEX = new RegExp("(" + WHITELISTED_ATTRIBUTES.join("|") + ")=['\"]?[^\\s\\]=]+['\"]?", "g"); Discourse.Dialect.replaceBlock({ start: /\[poll([^\]]*)\]([\s\S]*)/igm, @@ -45,7 +45,7 @@ // extract poll attributes (matches[1].match(ATTRIBUTES_REGEX) || []).forEach(function(m) { var attr = m.split("="), name = attr[0], value = attr[1]; - value = value.replace(/["']/g, ""); + value = Handlebars.Utils.escapeExpression(value.replace(/["']/g, "")); attributes[DATA_PREFIX + name] = value; }); @@ -98,12 +98,6 @@ contents[0][o].splice(1, 0, attr); } - // // add some information when type is "multiple" - // if (attributes[DATA_PREFIX + "type"] === "multiple") { - - - // } - var result = ["div", attributes], poll = ["div"]; diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 3808e0aaa59..ead1b6122d3 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -220,7 +220,7 @@ after_initialize do if polls.has_key?(poll["name"]) poll["name"] == DEFAULT_POLL_NAME ? self.errors.add(:base, I18n.t("poll.multiple_polls_without_name")) : - self.errors.add(:base, I18n.t("poll.multiple_polls_with_same_name", name: ERB::Util.html_escape(poll["name"]))) + self.errors.add(:base, I18n.t("poll.multiple_polls_with_same_name", name: poll["name"])) return end @@ -228,7 +228,7 @@ after_initialize do if poll["options"].map { |o| o["id"] }.uniq.size != poll["options"].size poll["name"] == DEFAULT_POLL_NAME ? self.errors.add(:base, I18n.t("poll.default_poll_must_have_different_options")) : - self.errors.add(:base, I18n.t("poll.named_poll_must_have_different_options", name: ERB::Util.html_escape(poll["name"]))) + self.errors.add(:base, I18n.t("poll.named_poll_must_have_different_options", name: poll["name"])) return end @@ -236,7 +236,7 @@ after_initialize do if poll["options"].size < 2 poll["name"] == DEFAULT_POLL_NAME ? self.errors.add(:base, I18n.t("poll.default_poll_must_have_at_least_2_options")) : - self.errors.add(:base, I18n.t("poll.named_poll_must_have_at_least_2_options", name: ERB::Util.html_escape(poll["name"]))) + self.errors.add(:base, I18n.t("poll.named_poll_must_have_at_least_2_options", name: poll["name"])) return end @@ -244,7 +244,7 @@ after_initialize do if poll["options"].size > SiteSetting.poll_maximum_options poll["name"] == DEFAULT_POLL_NAME ? self.errors.add(:base, I18n.t("poll.default_poll_must_have_less_options", max: SiteSetting.poll_maximum_options)) : - self.errors.add(:base, I18n.t("poll.named_poll_must_have_less_options", name: ERB::Util.html_escape(poll["name"]), max: SiteSetting.poll_maximum_options)) + self.errors.add(:base, I18n.t("poll.named_poll_must_have_less_options", name: poll["name"], max: SiteSetting.poll_maximum_options)) return end diff --git a/plugins/poll/spec/controllers/posts_controller_spec.rb b/plugins/poll/spec/controllers/posts_controller_spec.rb index 780789129e2..3d63d8425bc 100644 --- a/plugins/poll/spec/controllers/posts_controller_spec.rb +++ b/plugins/poll/spec/controllers/posts_controller_spec.rb @@ -49,6 +49,14 @@ describe PostsController do expect(json["errors"][0]).to eq(I18n.t("poll.default_poll_must_have_less_options", max: SiteSetting.poll_maximum_options)) end + it "prevents self-xss" do + xhr :post, :create, { title: title, raw: "[poll name=]\n- A\n- B\n[/poll]" } + expect(response).to be_success + json = ::JSON.parse(response.body) + expect(json["cooked"]).to match("data-poll-") + expect(json["polls"]["<script>alert(xss)</script>"]).to be + end + describe "edit window" do describe "within the first 5 minutes" do