From 2719b9e81994e961bf8c4e12b4556dc9777dd62f Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Mon, 13 Jun 2022 11:13:56 -0600 Subject: [PATCH] SECURITY: correctly escape event name (#280) --- .../discourse/widgets/discourse-post-event.js | 4 +- .../event-invitation-notification-item.js | 6 +- .../event-reminder-notification-item.js | 6 +- .../20220613073844_unescape_event_name.rb | 65 +++++++++++++++++++ lib/discourse_post_event/event_parser.rb | 6 +- .../discourse_post_event/event_parser_spec.rb | 5 ++ 6 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 db/post_migrate/20220613073844_unescape_event_name.rb diff --git a/assets/javascripts/discourse/widgets/discourse-post-event.js b/assets/javascripts/discourse/widgets/discourse-post-event.js index 2d31786b7..0a45842ea 100644 --- a/assets/javascripts/discourse/widgets/discourse-post-event.js +++ b/assets/javascripts/discourse/widgets/discourse-post-event.js @@ -10,6 +10,7 @@ import { createWidget } from "discourse/widgets/widget"; import { routeAction } from "discourse/helpers/route-action"; import { buildParams, replaceRaw } from "../../lib/raw-event-helper"; import bootbox from "bootbox"; +import { escapeExpression } from "discourse/lib/utilities"; export default createWidget("discourse-post-event", { tagName: "div.discourse-post-event-widget", @@ -160,7 +161,7 @@ export default createWidget("discourse-post-event", { startsAtMonth: moment(eventModel.starts_at).format("MMM"), startsAtDay: moment(eventModel.starts_at).format("D"), eventName: emojiUnescape( - eventModel.name || + escapeExpression(eventModel.name) || this._cleanTopicTitle( eventModel.post.topic.title, eventModel.starts_at @@ -257,6 +258,7 @@ export default createWidget("discourse-post-event", { `, _cleanTopicTitle(topicTitle, startsAt) { + topicTitle = escapeExpression(topicTitle); const cleaned = cleanTitle(topicTitle, startsAt); if (cleaned) { return topicTitle.replace(cleaned, ""); diff --git a/assets/javascripts/discourse/widgets/event-invitation-notification-item.js b/assets/javascripts/discourse/widgets/event-invitation-notification-item.js index e23f38937..5806588fb 100644 --- a/assets/javascripts/discourse/widgets/event-invitation-notification-item.js +++ b/assets/javascripts/discourse/widgets/event-invitation-notification-item.js @@ -1,7 +1,7 @@ import I18n from "I18n"; import { createWidgetFrom } from "discourse/widgets/widget"; import { DefaultNotificationItem } from "discourse/widgets/default-notification-item"; -import { formatUsername } from "discourse/lib/utilities"; +import { escapeExpression, formatUsername } from "discourse/lib/utilities"; import { iconNode } from "discourse-common/lib/icon-library"; createWidgetFrom( @@ -16,7 +16,9 @@ createWidgetFrom( const username = `${formatUsername(data.display_username)}`; let description; if (data.topic_title) { - description = `${data.topic_title}`; + description = `${escapeExpression(data.topic_title)}`; } else { description = this.description(data); } diff --git a/assets/javascripts/discourse/widgets/event-reminder-notification-item.js b/assets/javascripts/discourse/widgets/event-reminder-notification-item.js index 7e785ab9e..7fd8b0ae1 100644 --- a/assets/javascripts/discourse/widgets/event-reminder-notification-item.js +++ b/assets/javascripts/discourse/widgets/event-reminder-notification-item.js @@ -1,7 +1,7 @@ import I18n from "I18n"; import { createWidgetFrom } from "discourse/widgets/widget"; import { DefaultNotificationItem } from "discourse/widgets/default-notification-item"; -import { formatUsername } from "discourse/lib/utilities"; +import { escapeExpression, formatUsername } from "discourse/lib/utilities"; import { iconNode } from "discourse-common/lib/icon-library"; createWidgetFrom(DefaultNotificationItem, "event-reminder-notification-item", { @@ -14,7 +14,9 @@ createWidgetFrom(DefaultNotificationItem, "event-reminder-notification-item", { let description; if (data.topic_title) { - description = `${data.topic_title}`; + description = `${escapeExpression(data.topic_title)}`; } else { description = this.description(data); } diff --git a/db/post_migrate/20220613073844_unescape_event_name.rb b/db/post_migrate/20220613073844_unescape_event_name.rb new file mode 100644 index 000000000..0bf77bcd8 --- /dev/null +++ b/db/post_migrate/20220613073844_unescape_event_name.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +class UnescapeEventName < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def up + # event notifications + start = 1 + limit = DB.query_single("SELECT MAX(id) FROM notifications WHERE notification_type IN (27, 28)").first.to_i + + notifications_query = <<~SQL + SELECT id, data + FROM notifications + WHERE + id >= :start AND + notification_type IN (27, 28) AND + data::json ->> 'topic_title' LIKE '%&%' + ORDER BY id ASC + LIMIT 1000 + SQL + while true + if start > limit + break + end + max_seen = -1 + DB.query(notifications_query, start: start).each do |record| + id = record.id + if id > max_seen + max_seen = id + end + data = JSON.parse(record.data) + unescaped = CGI.unescapeHTML(data["topic_title"]) + next if unescaped == data["topic_title"] + data["topic_title"] = unescaped + DB.exec(<<~SQL, data: data.to_json, id: id) + UPDATE notifications SET data = :data WHERE id = :id + SQL + end + start += 1000 + if max_seen > start + start = max_seen + 1 + end + end + + # event names + events_query = <<~SQL + SELECT id, name + FROM discourse_post_event_events + WHERE name LIKE '%&%' + ORDER BY id ASC + SQL + + DB.query(events_query).each do |event| + unescaped_name = CGI.unescapeHTML(event.name) + next if unescaped_name == event.name + DB.exec(<<~SQL, unescaped_name: unescaped_name, id: event.id) + UPDATE discourse_post_event_events SET name = :unescaped_name WHERE id = :id + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/discourse_post_event/event_parser.rb b/lib/discourse_post_event/event_parser.rb index e1d10ff10..9e65e07ef 100644 --- a/lib/discourse_post_event/event_parser.rb +++ b/lib/discourse_post_event/event_parser.rb @@ -34,7 +34,11 @@ def self.extract_events(post) if value && valid_options.include?(name) event ||= {} - event[name.sub('data-', '').to_sym] = CGI.escapeHTML(value) + event[name.sub('data-', '').to_sym] = if name == "data-name" + value + else + CGI.escapeHTML(value) + end end valid_custom_fields.each do |valid_custom_field| diff --git a/spec/lib/discourse_post_event/event_parser_spec.rb b/spec/lib/discourse_post_event/event_parser_spec.rb index 12c38543c..18a83a865 100644 --- a/spec/lib/discourse_post_event/event_parser_spec.rb +++ b/spec/lib/discourse_post_event/event_parser_spec.rb @@ -80,6 +80,11 @@ def build_post(user, raw) expect(events).to eq([]) end + it 'doesn’t escape event name' do + events = subject.extract_events(build_post(user, '[event start="foo" name="bar