Commit e5bba18f authored by Steve Azzopardi's avatar Steve Azzopardi

Merge branch 'security-2717-xss-username-autocomplete' into 'master'

[master] Escape user fullname while rendering autocomplete template to prevent XSS

See merge request gitlab/gitlabhq!2605
parents b1405787 62be457f
......@@ -151,10 +151,16 @@ class GfmAutoComplete {
// Team Members
$input.atwho({
at: '@',
alias: 'users',
displayTpl(value) {
let tmpl = GfmAutoComplete.Loading.template;
if (value.username != null) {
tmpl = GfmAutoComplete.Members.template;
const { avatarTag, username, title } = value;
if (username != null) {
tmpl = GfmAutoComplete.Members.templateFunction({
avatarTag,
username,
title,
});
}
return tmpl;
},
......@@ -565,8 +571,9 @@ GfmAutoComplete.Emoji = {
};
// Team Members
GfmAutoComplete.Members = {
// eslint-disable-next-line no-template-curly-in-string
template: '<li>${avatarTag} ${username} <small>${title}</small></li>',
templateFunction({ avatarTag, username, title }) {
return `<li>${avatarTag} ${username} <small>${_.escape(title)}</small></li>`;
},
};
GfmAutoComplete.Labels = {
template:
......
---
title: Escape user fullname while rendering autocomplete template to prevent XSS
merge_request:
author:
type: security
require 'rails_helper'
describe 'GFM autocomplete', :js do
let(:issue_xss_title) { 'This will execute alert<img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
let(:user_xss_title) { 'eve <img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
let(:user_xss) { create(:user, name: user_xss_title, username: 'xss.user') }
let(:user) { create(:user, name: '💃speciąl someone💃', username: 'someone.special') }
let(:project) { create(:project) }
let(:label) { create(:label, project: project, title: 'special+') }
......@@ -9,6 +13,8 @@ describe 'GFM autocomplete', :js do
before do
project.add_maintainer(user)
project.add_maintainer(user_xss)
sign_in(user)
visit project_issue_path(project, issue)
......@@ -35,9 +41,8 @@ describe 'GFM autocomplete', :js do
expect(page).to have_selector('.atwho-container')
end
it 'opens autocomplete menu when field starts with text with item escaping HTML characters' do
alert_title = 'This will execute alert<img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;'
create(:issue, project: project, title: alert_title)
it 'opens autocomplete menu for Issues when field starts with text with item escaping HTML characters' do
create(:issue, project: project, title: issue_xss_title)
page.within '.timeline-content-form' do
find('#note-body').native.send_keys('#')
......@@ -46,7 +51,19 @@ describe 'GFM autocomplete', :js do
expect(page).to have_selector('.atwho-container')
page.within '.atwho-container #at-view-issues' do
expect(page.all('li').first.text).to include(alert_title)
expect(page.all('li').first.text).to include(issue_xss_title)
end
end
it 'opens autocomplete menu for Username when field starts with text with item escaping HTML characters' do
page.within '.timeline-content-form' do
find('#note-body').native.send_keys('@ev')
end
expect(page).to have_selector('.atwho-container')
page.within '.atwho-container #at-view-users' do
expect(find('li').text).to have_content(user_xss.username)
end
end
......@@ -107,7 +124,7 @@ describe 'GFM autocomplete', :js do
wait_for_requests
expect(find('#at-view-64')).to have_selector('.cur:first-of-type')
expect(find('#at-view-users')).to have_selector('.cur:first-of-type')
end
it 'includes items for assignee dropdowns with non-ASCII characters in name' do
......@@ -120,7 +137,7 @@ describe 'GFM autocomplete', :js do
wait_for_requests
expect(find('#at-view-64')).to have_content(user.name)
expect(find('#at-view-users')).to have_content(user.name)
end
it 'selects the first item for non-assignee dropdowns if a query is entered' do
......
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