Merge branch 'security-474-stored-xss-in-gfm-auto-complete' into 'master'

Fix stored XSS in GFM auto-complete

See merge request gitlab-org/security/gitlab!1703

app/assets/javascripts/gfm_auto_complete.js

 import $ from 'jquery';
 import '~/lib/utils/jquery_at_who';
-import { escape, sortBy, template } from 'lodash';
+import { escape as lodashEscape, sortBy, template } from 'lodash';
 import * as Emoji from '~/emoji';
 import axios from '~/lib/utils/axios_utils';
 import { s__, __, sprintf } from '~/locale';
@@ -11,8 +11,21 @@ import { spriteIcon } from './lib/utils/common_utils';
 import { parsePikadayDate } from './lib/utils/datetime_utility';
 import glRegexp from './lib/utils/regexp';
 
-function sanitize(str) {
-  return str.replace(/<(?:.|\n)*?>/gm, '');
+/**
+ * Escapes user input before we pass it to at.js, which
+ * renders it as HTML in the autocomplete dropdown.
+ *
+ * at.js allows you to reference data using `${}` syntax
+ * (e.g. ${search}) which it replaces with the actual data
+ * before rendering it in the autocomplete dropdown.
+ * To prevent user input from executing this `${}` syntax,
+ * we also need to escape the $ character.
+ *
+ * @param string user input
+ * @return {string} escaped user input
+ */
+function escape(string) {
+  return lodashEscape(string).replace(/\$/g, '&dollar;');
 }
 
 function createMemberSearchString(member) {
@@ -44,8 +57,8 @@ export function membersBeforeSave(members) {
     return {
       username: member.username,
       avatarTag: autoCompleteAvatar.length === 1 ? txtAvatar : imgAvatar,
-      title: sanitize(title),
-      search: sanitize(createMemberSearchString(member)),
+      title,
+      search: createMemberSearchString(member),
       icon: avatarIcon,
       availability: member?.availability,
     };
@@ -366,7 +379,7 @@ class GfmAutoComplete {
             }
             return {
               id: i.iid,
-              title: sanitize(i.title),
+              title: i.title,
               reference: i.reference,
               search: `${i.iid} ${i.title}`,
             };
@@ -404,7 +417,7 @@ class GfmAutoComplete {
 
             return {
               id: m.iid,
-              title: sanitize(m.title),
+              title: m.title,
               search: m.title,
               expired,
               dueDate,
@@ -456,7 +469,7 @@ class GfmAutoComplete {
             }
             return {
               id: m.iid,
-              title: sanitize(m.title),
+              title: m.title,
               reference: m.reference,
               search: `${m.iid} ${m.title}`,
             };
@@ -492,7 +505,7 @@ class GfmAutoComplete {
         beforeSave(merges) {
           if (GfmAutoComplete.isLoading(merges)) return merges;
           return $.map(merges, (m) => ({
-            title: sanitize(m.title),
+            title: m.title,
             color: m.color,
             search: m.title,
             set: m.set,
@@ -586,7 +599,7 @@ class GfmAutoComplete {
             }
             return {
               id: m.id,
-              title: sanitize(m.title),
+              title: m.title,
               search: `${m.id} ${m.title}`,
             };
           });

ee/app/assets/javascripts/gfm_auto_complete.js

@@ -48,7 +48,7 @@ class GfmAutoCompleteEE extends GfmAutoComplete {
             return {
               id: m.iid,
               reference: m.reference,
-              title: m.title.replace(/<(?:.|\n)*?>/gm, ''),
+              title: m.title,
               search: `${m.iid} ${m.title}`,
             };
           });
@@ -82,7 +82,7 @@ class GfmAutoCompleteEE extends GfmAutoComplete {
             }
             return {
               id: m.id,
-              title: m.title.replace(/<(?:.|\n)*?>/gm, ''),
+              title: m.title,
               reference: m.reference,
               search: `${m.id} ${m.title}`,
             };

spec/frontend/gfm_auto_complete_spec.js

@@ -574,6 +574,15 @@ describe('GfmAutoComplete', () => {
         }),
       ).toBe('<li><small>grp/proj#5</small> Some Issue</li>');
     });
+
+    it('escapes title in the template as it is user input', () => {
+      expect(
+        GfmAutoComplete.Issues.templateFunction({
+          id: 5,
+          title: '${search}<script>oh no $', // eslint-disable-line no-template-curly-in-string
+        }),
+      ).toBe('<li><small>5</small> &dollar;{search}&lt;script&gt;oh no &dollar;</li>');
+    });
   });
 
   describe('GfmAutoComplete.Members', () => {
@@ -608,16 +617,18 @@ describe('GfmAutoComplete', () => {
         ).toBe('<li>IMG my-group <small></small> <i class="icon"/></li>');
       });
 
-      it('should add escaped title if title is set', () => {
+      it('escapes title in the template as it is user input', () => {
         expect(
           GfmAutoComplete.Members.templateFunction({
             avatarTag: 'IMG',
             username: 'my-group',
-            title: 'MyGroup+',
+            title: '${search}<script>oh no $', // eslint-disable-line no-template-curly-in-string
             icon: '<i class="icon"/>',
             availabilityStatus: '',
           }),
-        ).toBe('<li>IMG my-group <small>MyGroup+</small> <i class="icon"/></li>');
+        ).toBe(
+          '<li>IMG my-group <small>&dollar;{search}&lt;script&gt;oh no &dollar;</small> <i class="icon"/></li>',
+        );
       });
 
       it('should add user availability status if availabilityStatus is set', () => {
@@ -782,6 +793,15 @@ describe('GfmAutoComplete', () => {
         ${'/unlabel ~'} | ${assignedLabels}
       `('$input shows $output.length labels', expectLabels);
     });
+
+    it('escapes title in the template as it is user input', () => {
+      const color = '#123456';
+      const title = '${search}<script>oh no $'; // eslint-disable-line no-template-curly-in-string
+
+      expect(GfmAutoComplete.Labels.templateFunction(color, title)).toBe(
+        '<li><span class="dropdown-label-box" style="background: #123456"></span> &dollar;{search}&lt;script&gt;oh no &dollar;</li>',
+      );
+    });
   });
 
   describe('emoji', () => {
@@ -829,4 +849,15 @@ describe('GfmAutoComplete', () => {
       });
     });
   });
+
+  describe('milestones', () => {
+    it('escapes title in the template as it is user input', () => {
+      const expired = false;
+      const title = '${search}<script>oh no $'; // eslint-disable-line no-template-curly-in-string
+
+      expect(GfmAutoComplete.Milestones.templateFunction(title, expired)).toBe(
+        '<li>&dollar;{search}&lt;script&gt;oh no &dollar;</li>',
+      );
+    });
+  });
 });