From 1cc53da1cfdb3d98dda58716d21dc66ba7eb4154 Mon Sep 17 00:00:00 2001
From: Thomas Randolph <trandolph@gitlab.com>
Date: Tue, 15 Mar 2022 17:16:16 -0600
Subject: [PATCH] Use Lodash `escape` as a small security enhancement

There's no _known_ way this could be exploited:
- Username and Display name are both restricted on save
- Most of the data passes through `<%-%>` anyway, which escapes it

This is mostly just a "cheap" (escape is easy) way to protect
against any accidents.
---
 app/assets/javascripts/users_select/index.js |  6 ++--
 spec/frontend/users_select/index_spec.js     | 35 ++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/users_select/index.js b/app/assets/javascripts/users_select/index.js
index 8ed92e6b948de1..656c851aa3d366 100644
--- a/app/assets/javascripts/users_select/index.js
+++ b/app/assets/javascripts/users_select/index.js
@@ -210,7 +210,7 @@ function UsersSelect(currentUser, els, options = {}) {
 
       return axios.put(issueURL, data).then(({ data }) => {
         let user = {};
-        let tooltipTitle = user.name;
+        let tooltipTitle;
         $dropdown.trigger('loaded.gl.dropdown');
         $loading.addClass('gl-display-none');
         if (data.assignee) {
@@ -806,7 +806,9 @@ UsersSelect.prototype.renderRow = function (
           </strong>
           ${
             username
-              ? `<span class="dropdown-menu-user-username gl-text-gray-400">${username}</span>`
+              ? `<span class="dropdown-menu-user-username gl-text-gray-400">${escape(
+                  username,
+                )}</span>`
               : ''
           }
           ${this.renderApprovalRules(elsClassName, user.applicable_approval_rules)}
diff --git a/spec/frontend/users_select/index_spec.js b/spec/frontend/users_select/index_spec.js
index 0d2aae78944993..3757e63c4f9627 100644
--- a/spec/frontend/users_select/index_spec.js
+++ b/spec/frontend/users_select/index_spec.js
@@ -108,4 +108,39 @@ describe('~/users_select/index', () => {
       });
     });
   });
+
+  describe('XSS', () => {
+    const escaped = '&gt;&lt;script&gt;alert(1)&lt;/script&gt;';
+    const issuableType = 'merge_request';
+    const user = {
+      availability: 'not_set',
+      can_merge: true,
+      name: 'name',
+    };
+    const selected = true;
+    const username = 'username';
+    const img = '<img user-avatar />';
+    const elsClassName = 'elsclass';
+
+    it.each`
+      prop          | val                             | element
+      ${'username'} | ${'><script>alert(1)</script>'} | ${'.dropdown-menu-user-username'}
+      ${'name'}     | ${'><script>alert(1)</script>'} | ${'.dropdown-menu-user-full-name'}
+    `('properly escapes the $prop $val', ({ prop, val, element }) => {
+      const u = prop === 'username' ? val : username;
+      const n = prop === 'name' ? val : user.name;
+      const row = UsersSelect.prototype.renderRow(
+        issuableType,
+        { ...user, name: n },
+        selected,
+        u,
+        img,
+        elsClassName,
+      );
+      const fragment = document.createRange().createContextualFragment(row);
+      const output = fragment.querySelector(element).innerHTML.trim();
+
+      expect(output).toBe(escaped);
+    });
+  });
 });
-- 
GitLab