Commit 117072d4 authored by Kushal Pandya's avatar Kushal Pandya

Fix user name autocomplete XSS when name contains HTML

parent 2e690c82
...@@ -151,10 +151,16 @@ class GfmAutoComplete { ...@@ -151,10 +151,16 @@ class GfmAutoComplete {
// Team Members // Team Members
$input.atwho({ $input.atwho({
at: '@', at: '@',
alias: 'users',
displayTpl(value) { displayTpl(value) {
let tmpl = GfmAutoComplete.Loading.template; let tmpl = GfmAutoComplete.Loading.template;
if (value.username != null) { const { avatarTag, username, title } = value;
tmpl = GfmAutoComplete.Members.template; if (username != null) {
tmpl = GfmAutoComplete.Members.templateFunction({
avatarTag,
username,
title,
});
} }
return tmpl; return tmpl;
}, },
...@@ -565,8 +571,9 @@ GfmAutoComplete.Emoji = { ...@@ -565,8 +571,9 @@ GfmAutoComplete.Emoji = {
}; };
// Team Members // Team Members
GfmAutoComplete.Members = { GfmAutoComplete.Members = {
// eslint-disable-next-line no-template-curly-in-string templateFunction({ avatarTag, username, title }) {
template: '<li>${avatarTag} ${username} <small>${title}</small></li>', return `<li>${avatarTag} ${username} <small>${_.escape(title)}</small></li>`;
},
}; };
GfmAutoComplete.Labels = { GfmAutoComplete.Labels = {
template: template:
......
require 'rails_helper' require 'rails_helper'
describe 'GFM autocomplete', :js do 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(:user) { create(:user, name: '💃speciąl someone💃', username: 'someone.special') }
let(:project) { create(:project) } let(:project) { create(:project) }
let(:label) { create(:label, project: project, title: 'special+') } let(:label) { create(:label, project: project, title: 'special+') }
...@@ -9,6 +13,8 @@ describe 'GFM autocomplete', :js do ...@@ -9,6 +13,8 @@ describe 'GFM autocomplete', :js do
before do before do
project.add_maintainer(user) project.add_maintainer(user)
project.add_maintainer(user_xss)
sign_in(user) sign_in(user)
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
...@@ -35,9 +41,8 @@ describe 'GFM autocomplete', :js do ...@@ -35,9 +41,8 @@ describe 'GFM autocomplete', :js do
expect(page).to have_selector('.atwho-container') expect(page).to have_selector('.atwho-container')
end end
it 'opens autocomplete menu when field starts with text with item escaping HTML characters' do it 'opens autocomplete menu for Issues 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: issue_xss_title)
create(:issue, project: project, title: alert_title)
page.within '.timeline-content-form' do page.within '.timeline-content-form' do
find('#note-body').native.send_keys('#') find('#note-body').native.send_keys('#')
...@@ -46,7 +51,19 @@ describe 'GFM autocomplete', :js do ...@@ -46,7 +51,19 @@ describe 'GFM autocomplete', :js do
expect(page).to have_selector('.atwho-container') expect(page).to have_selector('.atwho-container')
page.within '.atwho-container #at-view-issues' do 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
end end
...@@ -107,7 +124,7 @@ describe 'GFM autocomplete', :js do ...@@ -107,7 +124,7 @@ describe 'GFM autocomplete', :js do
wait_for_requests 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 end
it 'includes items for assignee dropdowns with non-ASCII characters in name' do it 'includes items for assignee dropdowns with non-ASCII characters in name' do
...@@ -120,7 +137,7 @@ describe 'GFM autocomplete', :js do ...@@ -120,7 +137,7 @@ describe 'GFM autocomplete', :js do
wait_for_requests wait_for_requests
expect(find('#at-view-64')).to have_content(user.name) expect(find('#at-view-users')).to have_content(user.name)
end end
it 'selects the first item for non-assignee dropdowns if a query is entered' do 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