CVE-2018-9986: Fix XSS issues with frontend markdown processor.

This fixes a set of XSS issues with Zulip's frontend markdown
processor, which is used in a limited set of contexts, such as local
echo of messages and the drafts feature.

The implementation of several syntax elements, including the <em>
syntax, user and stream mentions, and some others failed to properly
escape the content inside the syntax.

Fix this, and add tests for each corrected code path.

Thanks to w2w for reporting this issue.

frontend_tests/node_tests/markdown.js

@@ -76,6 +76,12 @@ people.add({
     email: 'leo@zulip.com',
 });
 
+people.add({
+    full_name: 'Bobby <h1>Tables</h1>',
+    user_id: 103,
+    email: 'bobby@zulip.com',
+});
+
 people.initialize_current_user(cordelia.user_id);
 
 var hamletcharacters = {
@@ -92,8 +98,16 @@ var backend = {
     members: [],
 };
 
+var edgecase_group = {
+    name: "Bobby <h1>Tables</h1>",
+    id: 3,
+    description: "HTML Syntax to check for Markdown edge cases.",
+    members: [],
+};
+
 global.user_groups.add(hamletcharacters);
 global.user_groups.add(backend);
+global.user_groups.add(edgecase_group);
 
 var stream_data = global.stream_data;
 var denmark = {
@@ -111,8 +125,16 @@ var social = {
     in_home_view: true,
     invite_only: true,
 };
+var edgecase_stream = {
+    subscribed: true,
+    color: 'green',
+    name: 'Bobby <h1>Tables</h1>',
+    stream_id: 3,
+    in_home_view: true,
+};
 stream_data.add_sub('Denmark', denmark);
 stream_data.add_sub('social', social);
+stream_data.add_sub('Bobby <h1>Tables</h1>', edgecase_stream);
 
 // Check the default behavior of fenced code blocks
 // works properly before markdown is initialized.
@@ -305,6 +327,23 @@ var bugdown_data = JSON.parse(fs.readFileSync(path.join(__dirname, '../../zerver
         {input: ':)',
          expected: '<p><span class="emoji emoji-1f603" title="smiley">:smiley:</span></p>',
          translate_emoticons: true},
+         // Test HTML Escape in Custom Zulip Rules
+        {input: '@**<h1>The Rogue One</h1>**',
+         expected: '<p>@**&lt;h1&gt;The Rogue One&lt;/h1&gt;**</p>'},
+        {input: '#**<h1>The Rogue One</h1>**',
+         expected: '<p>#**&lt;h1&gt;The Rogue One&lt;/h1&gt;**</p>'},
+        {input: '!avatar(<h1>The Rogue One</h1>)',
+         expected: '<p><img alt="&lt;h1&gt;The Rogue One&lt;/h1&gt;" class="message_body_gravatar" src="/avatar/&lt;h1&gt;The Rogue One&lt;/h1&gt;?s=30" title="&lt;h1&gt;The Rogue One&lt;/h1&gt;"></p>'},
+        {input: ':<h1>The Rogue One</h1>:',
+         expected: '<p>:&lt;h1&gt;The Rogue One&lt;/h1&gt;:</p>'},
+        {input: '@**O\'Connell**',
+         expected: '<p>@**O&#39;Connell**</p>'},
+        {input: '@*Bobby <h1>Tables</h1>*',
+         expected: '<p><span class="user-group-mention" data-user-group-id="3">@Bobby &lt;h1&gt;Tables&lt;/h1&gt;</span></p>'},
+        {input: '@**Bobby <h1>Tables</h1>**',
+         expected: '<p><span class="user-mention" data-user-id="103">@Bobby &lt;h1&gt;Tables&lt;/h1&gt;</span></p>'},
+        {input: '#**Bobby <h1>Tables</h1>**',
+         expected: '<p><a class="stream" data-stream-id="3" href="http://zulip.zulipdev.com/#narrow/stream/3-Bobby-.3Ch1.3ETables.3C.2Fh1.3E">#Bobby &lt;h1&gt;Tables&lt;/h1&gt;</a></p>'},
     ];
 
     // We remove one of the unicode emoji we put as input in one of the test
@@ -322,7 +361,6 @@ var bugdown_data = JSON.parse(fs.readFileSync(path.join(__dirname, '../../zerver
         var message = {raw_content: input};
         markdown.apply_markdown(message);
         var output = message.content;
-
         assert.equal(expected, output);
     });
 }());

static/js/markdown.js

@@ -11,6 +11,17 @@ var exports = {};
 var realm_filter_map = {};
 var realm_filter_list = [];
 
+
+// Helper function
+function escape(html, encode) {
+  return html
+    .replace(!encode ? /&(?!#?\w+;)/g : /&/g, '&')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;');
+}
+
 // Regexes that match some of our common bugdown markup
 var backend_only_markdown_re = [
     // Inline image previews, check for contiguous chars ending in image suffix
@@ -55,7 +66,7 @@ exports.apply_markdown = function (message) {
                     message.mentioned_me_directly = true;
                 }
                 return '<span class="user-mention" data-user-id="' + person.user_id + '">' +
-                       '@' + person.full_name +
+                       '@' + escape(person.full_name, true) +
                        '</span>';
             } else if (name === 'all' || name === 'everyone' || name === 'stream') {
                 message.mentioned = true;
@@ -72,7 +83,7 @@ exports.apply_markdown = function (message) {
                     message.mentioned = true;
                 }
                 return '<span class="user-group-mention" data-user-group-id="' + group.id + '">' +
-                       '@' + group.name +
+                       '@' + escape(group.name, true) +
                        '</span>';
             }
             return;
@@ -117,15 +128,6 @@ exports.is_status_message = function (raw_content, content) {
             content.lastIndexOf('</p>') === content.length - 4);
 };
 
-function escape(html, encode) {
-  return html
-    .replace(!encode ? /&(?!#?\w+;)/g : /&/g, '&amp;')
-    .replace(/</g, '&lt;')
-    .replace(/>/g, '&gt;')
-    .replace(/"/g, '&quot;')
-    .replace(/'/g, '&#39;');
-}
-
 function handleUnicodeEmoji(unicode_emoji) {
     var codepoint = unicode_emoji.codePointAt(0).toString(16);
     if (emoji_codes.codepoint_to_name.hasOwnProperty(codepoint)) {
@@ -170,7 +172,7 @@ function handleStream(streamName) {
     var href = window.location.origin + '/#narrow/stream/' + hash_util.encode_stream_name(stream.name);
     return '<a class="stream" data-stream-id="' + stream.stream_id + '" ' +
         'href="' + href + '"' +
-        '>' + '#' + stream.name + '</a>';
+        '>' + '#' + escape(stream.name) + '</a>';
 
 }
 

static/third/marked/lib/marked.js

@@ -736,7 +736,7 @@ InlineLexer.prototype.output = function(src) {
     // em
     if (cap = this.rules.em.exec(src)) {
       src = src.substring(cap[0].length);
-      out += this.renderer.em(cap[1] + cap[2]);
+      out += this.renderer.em(this.output(cap[1] + cap[2]));
       continue;
     }
 
@@ -825,6 +825,7 @@ InlineLexer.prototype.outputLink = function(cap, link) {
     : this.renderer.image(href, title, escape(cap[1]));
 };
 InlineLexer.prototype.emoji = function (name) {
+  name = escape(name)
   if (typeof this.options.emojiHandler !== 'function')
     return ':' + name + ':';
 
@@ -832,6 +833,7 @@ InlineLexer.prototype.emoji = function (name) {
 };
 
 InlineLexer.prototype.unicodeEmoji = function (name) {
+  name = escape(name)
   if (typeof this.options.unicodeEmojiHandler !== 'function')
     return name;
   return this.options.unicodeEmojiHandler(name);
@@ -844,12 +846,14 @@ InlineLexer.prototype.tex = function (tex, fullmatch) {
 };
 
 InlineLexer.prototype.userAvatar = function (email) {
+  email = escape(email);
   if (typeof this.options.avatarHandler !== 'function')
     return '!avatar(' + email + ')';
   return this.options.avatarHandler(email);
 };
 
 InlineLexer.prototype.userGravatar = function (email) {
+  email = escape(email);
   if (typeof this.options.avatarHandler !== 'function')
     return '!gravatar(' + email + ')';
   return this.options.avatarHandler(email);
@@ -863,6 +867,7 @@ InlineLexer.prototype.realm_filter = function (filter, matches, orig) {
 };
 
 InlineLexer.prototype.usermention = function (username, orig) {
+  orig = escape(orig);
   if (typeof this.options.userMentionHandler !== 'function')
   {
     return orig;
@@ -877,6 +882,7 @@ InlineLexer.prototype.usermention = function (username, orig) {
 };
 
 InlineLexer.prototype.groupmention = function (groupname, orig) {
+  orig = escape(orig);
   if (typeof this.options.groupMentionHandler !== 'function')
   {
     return orig;
@@ -891,6 +897,7 @@ InlineLexer.prototype.groupmention = function (groupname, orig) {
 };
 
 InlineLexer.prototype.stream = function (streamName, orig) {
+  orig = escape(orig);
   if (typeof this.options.streamHandler !== 'function')
     return orig;
 
@@ -1031,6 +1038,7 @@ Renderer.prototype.strong = function(text) {
 };
 
 Renderer.prototype.em = function(text) {
+  text = escape(text);
   return '<em>' + text + '</em>';
 };
 

zerver/fixtures/markdown_test_cases.json

@@ -216,6 +216,11 @@
       "expected_output": "<p>A <em>foo bar</em> is a <em>baz quux</em></p>",
       "text_content": "A foo bar is a baz quux"
     },
+    {
+      "name": "emphasis_with_html",
+      "input": "*<h1>Hello World</h1>*",
+      "expected_output": "<p><em>&lt;h1&gt;Hello World&lt;/h1&gt;</em></p>"
+    },
     {
       "name": "underscore_strong_disabled",
       "input": "__foo__",
@@ -233,6 +238,11 @@
       "expected_output": "<p><strong><em>foo</em></strong></p>",
       "text_content": "foo"
     },
+    {
+      "name": "strong_with_html",
+      "input": "**<h1>Hello World</h1>**",
+      "expected_output": "<p><strong>&lt;h1&gt;Hello World&lt;/h1&gt;</strong></p>"
+    },
     {
       "name": "numbered_list",
       "input": "1. A\n 2. B",