Commit 1cc53da1 authored by Thomas Randolph's avatar Thomas Randolph

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.
parent 3599bf45
...@@ -210,7 +210,7 @@ function UsersSelect(currentUser, els, options = {}) { ...@@ -210,7 +210,7 @@ function UsersSelect(currentUser, els, options = {}) {
return axios.put(issueURL, data).then(({ data }) => { return axios.put(issueURL, data).then(({ data }) => {
let user = {}; let user = {};
let tooltipTitle = user.name; let tooltipTitle;
$dropdown.trigger('loaded.gl.dropdown'); $dropdown.trigger('loaded.gl.dropdown');
$loading.addClass('gl-display-none'); $loading.addClass('gl-display-none');
if (data.assignee) { if (data.assignee) {
...@@ -806,7 +806,9 @@ UsersSelect.prototype.renderRow = function ( ...@@ -806,7 +806,9 @@ UsersSelect.prototype.renderRow = function (
</strong> </strong>
${ ${
username 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)} ${this.renderApprovalRules(elsClassName, user.applicable_approval_rules)}
......
...@@ -108,4 +108,39 @@ describe('~/users_select/index', () => { ...@@ -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);
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment