From d704472c6b45ec472287500bbe0c991aef2032bb Mon Sep 17 00:00:00 2001 From: vagrant Date: Thu, 3 Dec 2020 20:46:01 +0000 Subject: [PATCH] widgets: Add support for clickable links. The change was made to support links in polls, as mentioned in issue #12947. We used markdown renderer to render the link content, and parsed out any unnecessary p tags. We changed javascript and hbs files so that they properly render the content. Tested locally whether the links work, in addition to checking for XSS vulnerbilities. Everything tested worked, and no vulnerabilities discovered. Double check that there are no XSS issues. Fixes: #12947 --- static/js/poll_widget.js | 10 ++++------ static/templates/widgets/poll_widget_results.hbs | 2 +- zerver/lib/actions.py | 12 ++++++++++-- zerver/lib/widget.py | 15 ++++++++++++--- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/static/js/poll_widget.js b/static/js/poll_widget.js index 0dfe1ede9533a..74be0d7408604 100644 --- a/static/js/poll_widget.js +++ b/static/js/poll_widget.js @@ -59,7 +59,6 @@ class PollData { for (const [key, obj] of this.key_to_option) { const voters = Array.from(obj.votes.keys()); const current_user_vote = voters.includes(this.me); - options.push({ option: obj.option, names: people.safe_full_names(voters), @@ -87,7 +86,6 @@ class PollData { }; this.my_idx += 1; - return event; }, @@ -208,7 +206,7 @@ exports.activate = function (opts) { const author_help = is_my_poll && !has_question; elem.find(".poll-question-header").toggle(!input_mode); - elem.find(".poll-question-header").text(question); + elem.find(".poll-question-header").html(question); elem.find(".poll-edit-question").toggle(can_edit); update_edit_controls(); @@ -245,9 +243,9 @@ exports.activate = function (opts) { new_question = old_question; } - // Optimistically set the question locally. - poll_data.set_question(new_question); - render_question(); + // Decided not to do this to prevent XSS attacks, so that escaping could happen + // poll_data.set_question(new_question); + // render_question(); // If there were no actual edits, we can exit now. if (new_question === old_question) { diff --git a/static/templates/widgets/poll_widget_results.hbs b/static/templates/widgets/poll_widget_results.hbs index 64ca819002713..158e7d8eb72bf 100644 --- a/static/templates/widgets/poll_widget_results.hbs +++ b/static/templates/widgets/poll_widget_results.hbs @@ -3,7 +3,7 @@ - {{ option }} + {{{ option }}} {{#if names}} ({{ names }}) {{/if}} diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index b5ce2ba4ee07c..e9f7398ee8567 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -1,5 +1,6 @@ import datetime import itertools +import json import logging import os import time @@ -173,7 +174,7 @@ ) from zerver.lib.utils import generate_api_key, log_statsd_event from zerver.lib.validator import check_widget_content -from zerver.lib.widget import do_widget_post_save_actions +from zerver.lib.widget import do_widget_post_save_actions, filter_and_render_string from zerver.models import ( MAX_MESSAGE_LENGTH, Attachment, @@ -1803,6 +1804,14 @@ def do_add_submessage(realm: Realm, msg_type: str, content: str, ) -> None: + content_json = json.loads(content) + + if "option" in content_json: + content_json["option"] = filter_and_render_string(content_json["option"]) + elif "question" in content_json: + content_json["question"] = filter_and_render_string(content_json["question"]) + content = json.dumps(content_json) + submessage = SubMessage( sender_id=sender_id, message_id=message_id, @@ -1810,7 +1819,6 @@ def do_add_submessage(realm: Realm, content=content, ) submessage.save() - event = dict( type="submessage", msg_type=msg_type, diff --git a/zerver/lib/widget.py b/zerver/lib/widget.py index c87635b07cd99..e77e72848bbcb 100644 --- a/zerver/lib/widget.py +++ b/zerver/lib/widget.py @@ -2,9 +2,17 @@ import re from typing import Any, MutableMapping, Optional, Tuple -from zerver.models import SubMessage +from zerver.lib.markdown import markdown_convert +from zerver.models import SubMessage, get_realm +def filter_and_render_string(input: str) -> str: + # Run through the markdown engine so that links will work + output = markdown_convert(input, message_realm=get_realm('zulip'),) + # Remove p tags from render output, so the options do not create new lines + output = re.sub(r'<\/*p>', '', output) + return output + def get_widget_data(content: str) -> Tuple[Optional[str], Optional[str]]: valid_widget_types = ['poll', 'todo'] tokens = content.split(' ') @@ -28,17 +36,18 @@ def get_extra_data_from_widget_type(content: str, question = '' options = [] if lines and lines[0]: - question = lines.pop(0).strip() + question = filter_and_render_string(lines.pop(0).strip()) for line in lines: # If someone is using the list syntax, we remove it # before adding an option. option = re.sub(r'(\s*[-*]?\s*)', '', line.strip(), 1) if len(option) > 0: - options.append(option) + options.append(filter_and_render_string(option)) extra_data = { 'question': question, 'options': options, } + return extra_data return None