From ff06452e05b1191ce8649ae6a9e646341ab073ba Mon Sep 17 00:00:00 2001 From: Felipe Artur Date: Mon, 8 Apr 2019 15:21:21 -0300 Subject: [PATCH] Prevent leaking information when issue is moved Prevent leaking namespace and project names on moved issue links --- app/views/projects/issues/show.html.haml | 2 +- changelogs/unreleased/security-issue_2830.yml | 5 ++++ .../projects/issues/show.html.haml_spec.rb | 27 ++++++++++++++----- 3 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/security-issue_2830.yml diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index 2c95ac6dbb3..dc01bfa24ca 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -15,7 +15,7 @@ .issuable-status-box.status-box.status-box-issue-closed{ class: issue_button_visibility(@issue, false) } = sprite_icon('mobile-issue-close', size: 16, css_class: 'd-block d-sm-none') .d-none.d-sm-block - - if @issue.moved? + - if @issue.moved? && can?(current_user, :read_issue, @issue.moved_to) - moved_link_start = "".html_safe - moved_link_end = ''.html_safe = s_('IssuableStatus|Closed (%{moved_link_start}moved%{moved_link_end})').html_safe % {moved_link_start: moved_link_start, diff --git a/changelogs/unreleased/security-issue_2830.yml b/changelogs/unreleased/security-issue_2830.yml new file mode 100644 index 00000000000..244e105f7d4 --- /dev/null +++ b/changelogs/unreleased/security-issue_2830.yml @@ -0,0 +1,5 @@ +--- +title: 'Resolve: moving an issue to private repo leaks namespace and project name' +merge_request: +author: +type: security diff --git a/spec/views/projects/issues/show.html.haml_spec.rb b/spec/views/projects/issues/show.html.haml_spec.rb index 1d9c6d36ad7..1ca9eaf8fdb 100644 --- a/spec/views/projects/issues/show.html.haml_spec.rb +++ b/spec/views/projects/issues/show.html.haml_spec.rb @@ -19,6 +19,7 @@ describe 'projects/issues/show' do context 'when the issue is closed' do before do allow(issue).to receive(:closed?).and_return(true) + allow(view).to receive(:current_user).and_return(user) end context 'when the issue was moved' do @@ -28,16 +29,30 @@ describe 'projects/issues/show' do issue.moved_to = new_issue end - it 'shows "Closed (moved)" if an issue has been moved' do - render + context 'when user can see the moved issue' do + before do + project.add_developer(user) + end - expect(rendered).to have_selector('.status-box-issue-closed:not(.hidden)', text: 'Closed (moved)') + it 'shows "Closed (moved)" if an issue has been moved' do + render + + expect(rendered).to have_selector('.status-box-issue-closed:not(.hidden)', text: 'Closed (moved)') + end + + it 'links "moved" to the new issue the original issue was moved to' do + render + + expect(rendered).to have_selector("a[href=\"#{issue_path(new_issue)}\"]", text: 'moved') + end end - it 'links "moved" to the new issue the original issue was moved to' do - render + context 'when user cannot see moved issue' do + it 'does not show moved issue link' do + render - expect(rendered).to have_selector("a[href=\"#{issue_path(new_issue)}\"]", text: 'moved') + expect(rendered).not_to have_selector("a[href=\"#{issue_path(new_issue)}\"]", text: 'moved') + end end end