Commit 5f03d26a authored by Kushal Pandya's avatar Kushal Pandya

Escape label and milestone titles to prevent XSS

parent 00096b52
...@@ -256,7 +256,7 @@ class GfmAutoComplete { ...@@ -256,7 +256,7 @@ class GfmAutoComplete {
displayTpl(value) { displayTpl(value) {
let tmpl = GfmAutoComplete.Loading.template; let tmpl = GfmAutoComplete.Loading.template;
if (value.title != null) { if (value.title != null) {
tmpl = GfmAutoComplete.Milestones.template; tmpl = GfmAutoComplete.Milestones.templateFunction(value.title);
} }
return tmpl; return tmpl;
}, },
...@@ -323,7 +323,7 @@ class GfmAutoComplete { ...@@ -323,7 +323,7 @@ class GfmAutoComplete {
searchKey: 'search', searchKey: 'search',
data: GfmAutoComplete.defaultLoadingData, data: GfmAutoComplete.defaultLoadingData,
displayTpl(value) { displayTpl(value) {
let tmpl = GfmAutoComplete.Labels.template; let tmpl = GfmAutoComplete.Labels.templateFunction(value.color, value.title);
if (GfmAutoComplete.isLoading(value)) { if (GfmAutoComplete.isLoading(value)) {
tmpl = GfmAutoComplete.Loading.template; tmpl = GfmAutoComplete.Loading.template;
} }
...@@ -588,9 +588,11 @@ GfmAutoComplete.Members = { ...@@ -588,9 +588,11 @@ GfmAutoComplete.Members = {
}, },
}; };
GfmAutoComplete.Labels = { GfmAutoComplete.Labels = {
template: templateFunction(color, title) {
// eslint-disable-next-line no-template-curly-in-string return `<li><span class="dropdown-label-box" style="background: ${_.escape(
'<li><span class="dropdown-label-box" style="background: ${color}"></span> ${title}</li>', color,
)}"></span> ${_.escape(title)}</li>`;
},
}; };
// Issues, MergeRequests and Snippets // Issues, MergeRequests and Snippets
GfmAutoComplete.Issues = { GfmAutoComplete.Issues = {
...@@ -600,8 +602,9 @@ GfmAutoComplete.Issues = { ...@@ -600,8 +602,9 @@ GfmAutoComplete.Issues = {
}; };
// Milestones // Milestones
GfmAutoComplete.Milestones = { GfmAutoComplete.Milestones = {
// eslint-disable-next-line no-template-curly-in-string templateFunction(title) {
template: '<li>${title}</li>', return `<li>${_.escape(title)}</li>`;
},
}; };
GfmAutoComplete.Loading = { GfmAutoComplete.Loading = {
template: template:
......
...@@ -3,6 +3,8 @@ require 'rails_helper' ...@@ -3,6 +3,8 @@ 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(: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_title) { 'eve <img src=x onerror=alert(2)&lt;img src=x onerror=alert(1)&gt;' }
let(:label_xss_title) { 'alert label &lt;img src=x onerror="alert(\'Hello xss\');" a'}
let(:milestone_xss_title) { 'alert milestone &lt;img src=x onerror="alert(\'Hello xss\');" a' }
let(:user_xss) { create(:user, name: user_xss_title, username: 'xss.user') } 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') }
...@@ -25,10 +27,14 @@ describe 'GFM autocomplete', :js do ...@@ -25,10 +27,14 @@ describe 'GFM autocomplete', :js do
simulate_input('#issue-description', "@#{user.name[0...3]}") simulate_input('#issue-description', "@#{user.name[0...3]}")
wait_for_requests
find('.atwho-view .cur').click find('.atwho-view .cur').click
click_button 'Save changes' click_button 'Save changes'
wait_for_requests
expect(find('.description')).to have_content(user.to_reference) expect(find('.description')).to have_content(user.to_reference)
end end
...@@ -47,6 +53,8 @@ describe 'GFM autocomplete', :js do ...@@ -47,6 +53,8 @@ describe 'GFM autocomplete', :js do
find('#note-body').native.send_keys('#') find('#note-body').native.send_keys('#')
end end
wait_for_requests
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
...@@ -59,6 +67,8 @@ describe 'GFM autocomplete', :js do ...@@ -59,6 +67,8 @@ describe 'GFM autocomplete', :js do
find('#note-body').native.send_keys('@ev') find('#note-body').native.send_keys('@ev')
end end
wait_for_requests
expect(page).to have_selector('.atwho-container') expect(page).to have_selector('.atwho-container')
page.within '.atwho-container #at-view-users' do page.within '.atwho-container #at-view-users' do
...@@ -66,6 +76,22 @@ describe 'GFM autocomplete', :js do ...@@ -66,6 +76,22 @@ describe 'GFM autocomplete', :js do
end end
end end
it 'opens autocomplete menu for Milestone when field starts with text with item escaping HTML characters' do
create(:milestone, project: project, title: milestone_xss_title)
page.within '.timeline-content-form' do
find('#note-body').native.send_keys('%')
end
wait_for_requests
expect(page).to have_selector('.atwho-container')
page.within '.atwho-container #at-view-milestones' do
expect(find('li').text).to have_content('alert milestone')
end
end
it 'doesnt open autocomplete menu character is prefixed with text' do it 'doesnt open autocomplete menu character is prefixed with text' do
page.within '.timeline-content-form' do page.within '.timeline-content-form' do
find('#note-body').native.send_keys('testing') find('#note-body').native.send_keys('testing')
...@@ -258,12 +284,28 @@ describe 'GFM autocomplete', :js do ...@@ -258,12 +284,28 @@ describe 'GFM autocomplete', :js do
let!(:bug) { create(:label, project: project, title: 'bug') } let!(:bug) { create(:label, project: project, title: 'bug') }
let!(:feature_proposal) { create(:label, project: project, title: 'feature proposal') } let!(:feature_proposal) { create(:label, project: project, title: 'feature proposal') }
it 'opens autocomplete menu for Labels when field starts with text with item escaping HTML characters' do
create(:label, project: project, title: label_xss_title)
note = find('#note-body')
# It should show all the labels on "~".
type(note, '~')
wait_for_requests
page.within '.atwho-container #at-view-labels' do
expect(find('.atwho-view-ul').text).to have_content('alert label')
end
end
context 'when no labels are assigned' do context 'when no labels are assigned' do
it 'shows labels' do it 'shows labels' do
note = find('#note-body') note = find('#note-body')
# It should show all the labels on "~". # It should show all the labels on "~".
type(note, '~') type(note, '~')
wait_for_requests
expect_labels(shown: [backend, bug, feature_proposal]) expect_labels(shown: [backend, bug, feature_proposal])
# It should show all the labels on "/label ~". # It should show all the labels on "/label ~".
...@@ -290,6 +332,7 @@ describe 'GFM autocomplete', :js do ...@@ -290,6 +332,7 @@ describe 'GFM autocomplete', :js do
# It should show all the labels on "~". # It should show all the labels on "~".
type(note, '~') type(note, '~')
wait_for_requests
expect_labels(shown: [backend, bug, feature_proposal]) expect_labels(shown: [backend, bug, feature_proposal])
# It should show only unset labels on "/label ~". # It should show only unset labels on "/label ~".
...@@ -316,6 +359,7 @@ describe 'GFM autocomplete', :js do ...@@ -316,6 +359,7 @@ describe 'GFM autocomplete', :js do
# It should show all the labels on "~". # It should show all the labels on "~".
type(note, '~') type(note, '~')
wait_for_requests
expect_labels(shown: [backend, bug, feature_proposal]) expect_labels(shown: [backend, bug, feature_proposal])
# It should show no labels on "/label ~". # It should show no labels on "/label ~".
......
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