FIX: prevent self-xss in poll attributes

This commit is contained in:
Régis Hanol 2015-05-11 20:09:17 +02:00
parent d337da532a
commit 3a9907f392
3 changed files with 14 additions and 12 deletions

View file

@ -8,7 +8,7 @@
const WHITELISTED_ATTRIBUTES = ["type", "name", "min", "max", "step", "order", "color", "background", "status"]; const WHITELISTED_ATTRIBUTES = ["type", "name", "min", "max", "step", "order", "color", "background", "status"];
const WHITELISTED_STYLES = ["color", "background"]; 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({ Discourse.Dialect.replaceBlock({
start: /\[poll([^\]]*)\]([\s\S]*)/igm, start: /\[poll([^\]]*)\]([\s\S]*)/igm,
@ -45,7 +45,7 @@
// extract poll attributes // extract poll attributes
(matches[1].match(ATTRIBUTES_REGEX) || []).forEach(function(m) { (matches[1].match(ATTRIBUTES_REGEX) || []).forEach(function(m) {
var attr = m.split("="), name = attr[0], value = attr[1]; 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; attributes[DATA_PREFIX + name] = value;
}); });
@ -98,12 +98,6 @@
contents[0][o].splice(1, 0, attr); contents[0][o].splice(1, 0, attr);
} }
// // add some information when type is "multiple"
// if (attributes[DATA_PREFIX + "type"] === "multiple") {
// }
var result = ["div", attributes], var result = ["div", attributes],
poll = ["div"]; poll = ["div"];

View file

@ -220,7 +220,7 @@ after_initialize do
if polls.has_key?(poll["name"]) if polls.has_key?(poll["name"])
poll["name"] == DEFAULT_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_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 return
end end
@ -228,7 +228,7 @@ after_initialize do
if poll["options"].map { |o| o["id"] }.uniq.size != poll["options"].size if poll["options"].map { |o| o["id"] }.uniq.size != poll["options"].size
poll["name"] == DEFAULT_POLL_NAME ? 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.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 return
end end
@ -236,7 +236,7 @@ after_initialize do
if poll["options"].size < 2 if poll["options"].size < 2
poll["name"] == DEFAULT_POLL_NAME ? 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.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 return
end end
@ -244,7 +244,7 @@ after_initialize do
if poll["options"].size > SiteSetting.poll_maximum_options if poll["options"].size > SiteSetting.poll_maximum_options
poll["name"] == DEFAULT_POLL_NAME ? 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.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 return
end end

View file

@ -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)) expect(json["errors"][0]).to eq(I18n.t("poll.default_poll_must_have_less_options", max: SiteSetting.poll_maximum_options))
end end
it "prevents self-xss" do
xhr :post, :create, { title: title, raw: "[poll name=<script>alert('xss')</script>]\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"]["&lt;script&gt;alert(xss)&lt;/script&gt;"]).to be
end
describe "edit window" do describe "edit window" do
describe "within the first 5 minutes" do describe "within the first 5 minutes" do