Escaped html characters
This commit is contained in:
parent
b44ccb283f
commit
beb8354b34
|
@ -32,9 +32,9 @@
|
||||||
%a{ href: "##{line_code}", data: { linenumber: link_text } }
|
%a{ href: "##{line_code}", data: { linenumber: link_text } }
|
||||||
%td.line_content.noteable_line{ class: type }<
|
%td.line_content.noteable_line{ class: type }<
|
||||||
- if email
|
- if email
|
||||||
%pre= line.text
|
%pre= line.rich_text
|
||||||
- else
|
- else
|
||||||
= diff_line_content(line.text)
|
= diff_line_content(line.rich_text)
|
||||||
|
|
||||||
- if line_discussions&.any?
|
- if line_discussions&.any?
|
||||||
- discussion_expanded = local_assigns.fetch(:discussion_expanded, line_discussions.any?(&:expanded?))
|
- discussion_expanded = local_assigns.fetch(:discussion_expanded, line_discussions.any?(&:expanded?))
|
||||||
|
|
|
@ -24,7 +24,7 @@
|
||||||
- discussion_left = discussions_left.try(:first)
|
- discussion_left = discussions_left.try(:first)
|
||||||
- if discussion_left && discussion_left.resolvable?
|
- if discussion_left && discussion_left.resolvable?
|
||||||
%diff-note-avatars{ "discussion-id" => discussion_left.id }
|
%diff-note-avatars{ "discussion-id" => discussion_left.id }
|
||||||
%td.line_content.parallel.noteable_line.left-side{ id: left_line_code, class: left.type }= diff_line_content(left.text)
|
%td.line_content.parallel.noteable_line.left-side{ id: left_line_code, class: left.type }= diff_line_content(left.rich_text)
|
||||||
- else
|
- else
|
||||||
%td.old_line.diff-line-num.empty-cell
|
%td.old_line.diff-line-num.empty-cell
|
||||||
%td.line_content.parallel.left-side
|
%td.line_content.parallel.left-side
|
||||||
|
@ -45,7 +45,7 @@
|
||||||
- discussion_right = discussions_right.try(:first)
|
- discussion_right = discussions_right.try(:first)
|
||||||
- if discussion_right && discussion_right.resolvable?
|
- if discussion_right && discussion_right.resolvable?
|
||||||
%diff-note-avatars{ "discussion-id" => discussion_right.id }
|
%diff-note-avatars{ "discussion-id" => discussion_right.id }
|
||||||
%td.line_content.parallel.noteable_line.right-side{ id: right_line_code, class: right.type }= diff_line_content(right.text)
|
%td.line_content.parallel.noteable_line.right-side{ id: right_line_code, class: right.type }= diff_line_content(right.rich_text)
|
||||||
- else
|
- else
|
||||||
%td.old_line.diff-line-num.empty-cell
|
%td.old_line.diff-line-num.empty-cell
|
||||||
%td.line_content.parallel.right-side
|
%td.line_content.parallel.right-side
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fixed persistent XSS rendering/escaping of diff location lines
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: security
|
|
@ -37,7 +37,7 @@ module Gitlab
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
diff_line.text = rich_line
|
diff_line.rich_text = rich_line
|
||||||
|
|
||||||
diff_line
|
diff_line
|
||||||
end
|
end
|
||||||
|
|
|
@ -85,7 +85,7 @@ module Gitlab
|
||||||
old_line: old_line,
|
old_line: old_line,
|
||||||
new_line: new_line,
|
new_line: new_line,
|
||||||
text: text,
|
text: text,
|
||||||
rich_text: rich_text || text,
|
rich_text: rich_text || CGI.escapeHTML(text),
|
||||||
meta_data: meta_positions
|
meta_data: meta_positions
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
|
@ -2,6 +2,7 @@ require 'rails_helper'
|
||||||
|
|
||||||
describe 'Merge request > User sees diff', :js do
|
describe 'Merge request > User sees diff', :js do
|
||||||
include ProjectForksHelper
|
include ProjectForksHelper
|
||||||
|
include RepoHelpers
|
||||||
|
|
||||||
let(:project) { create(:project, :public, :repository) }
|
let(:project) { create(:project, :public, :repository) }
|
||||||
let(:merge_request) { create(:merge_request, source_project: project) }
|
let(:merge_request) { create(:merge_request, source_project: project) }
|
||||||
|
@ -81,5 +82,58 @@ describe 'Merge request > User sees diff', :js do
|
||||||
expect(page).to have_selector('.js-cancel-fork-suggestion-button', count: 1)
|
expect(page).to have_selector('.js-cancel-fork-suggestion-button', count: 1)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
context 'when file contains html' do
|
||||||
|
let(:current_user) { project.owner }
|
||||||
|
let(:branch_name) {"test_branch"}
|
||||||
|
|
||||||
|
def create_file(branch_name, file_name, content)
|
||||||
|
Files::CreateService.new(
|
||||||
|
project,
|
||||||
|
current_user,
|
||||||
|
start_branch: branch_name,
|
||||||
|
branch_name: branch_name,
|
||||||
|
commit_message: "Create file",
|
||||||
|
file_path: file_name,
|
||||||
|
file_content: content
|
||||||
|
).execute
|
||||||
|
|
||||||
|
project.commit(branch_name)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'escapes any HTML special characters in the diff chunk header' do
|
||||||
|
file_content =
|
||||||
|
<<~CONTENT
|
||||||
|
function foo<input> {
|
||||||
|
let a = 1;
|
||||||
|
let b = 2;
|
||||||
|
let c = 3;
|
||||||
|
let d = 3;
|
||||||
|
}
|
||||||
|
CONTENT
|
||||||
|
|
||||||
|
new_file_content =
|
||||||
|
<<~CONTENT
|
||||||
|
function foo<input> {
|
||||||
|
let a = 1;
|
||||||
|
let b = 2;
|
||||||
|
let c = 3;
|
||||||
|
let x = 3;
|
||||||
|
}
|
||||||
|
CONTENT
|
||||||
|
|
||||||
|
file_name = 'xss_file.txt'
|
||||||
|
|
||||||
|
create_file('master', file_name, file_content)
|
||||||
|
merge_request = create(:merge_request, source_project: project)
|
||||||
|
create_file(merge_request.source_branch, file_name, new_file_content)
|
||||||
|
|
||||||
|
project.commit(merge_request.source_branch)
|
||||||
|
|
||||||
|
visit diffs_project_merge_request_path(project, merge_request)
|
||||||
|
|
||||||
|
expect(page).to have_text("function foo<input> {")
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -24,19 +24,19 @@ describe Gitlab::Diff::Highlight do
|
||||||
it 'highlights and marks unchanged lines' do
|
it 'highlights and marks unchanged lines' do
|
||||||
code = %Q{ <span id="LC7" class="line" lang="ruby"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n}
|
code = %Q{ <span id="LC7" class="line" lang="ruby"> <span class="k">def</span> <span class="nf">popen</span><span class="p">(</span><span class="n">cmd</span><span class="p">,</span> <span class="n">path</span><span class="o">=</span><span class="kp">nil</span><span class="p">)</span></span>\n}
|
||||||
|
|
||||||
expect(subject[2].text).to eq(code)
|
expect(subject[2].rich_text).to eq(code)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'highlights and marks removed lines' do
|
it 'highlights and marks removed lines' do
|
||||||
code = %Q{-<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n}
|
code = %Q{-<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="s2">"System commands must be given as an array of strings"</span></span>\n}
|
||||||
|
|
||||||
expect(subject[4].text).to eq(code)
|
expect(subject[4].rich_text).to eq(code)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'highlights and marks added lines' do
|
it 'highlights and marks added lines' do
|
||||||
code = %Q{+<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="no"><span class="idiff left">RuntimeError</span></span><span class="p"><span class="idiff">,</span></span><span class="idiff right"> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n}
|
code = %Q{+<span id="LC9" class="line" lang="ruby"> <span class="k">raise</span> <span class="no"><span class="idiff left">RuntimeError</span></span><span class="p"><span class="idiff">,</span></span><span class="idiff right"> </span><span class="s2">"System commands must be given as an array of strings"</span></span>\n}
|
||||||
|
|
||||||
expect(subject[5].text).to eq(code)
|
expect(subject[5].rich_text).to eq(code)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -69,8 +69,8 @@ describe Gitlab::Diff::Highlight do
|
||||||
it 'marks added lines' do
|
it 'marks added lines' do
|
||||||
code = %q{+ raise <span class="idiff left right">RuntimeError, </span>"System commands must be given as an array of strings"}
|
code = %q{+ raise <span class="idiff left right">RuntimeError, </span>"System commands must be given as an array of strings"}
|
||||||
|
|
||||||
expect(subject[5].text).to eq(code)
|
expect(subject[5].rich_text).to eq(code)
|
||||||
expect(subject[5].text).to be_html_safe
|
expect(subject[5].rich_text).to be_html_safe
|
||||||
end
|
end
|
||||||
|
|
||||||
context 'when the inline diff marker has an invalid range' do
|
context 'when the inline diff marker has an invalid range' do
|
||||||
|
|
|
@ -0,0 +1,10 @@
|
||||||
|
describe Gitlab::Diff::Line do
|
||||||
|
context "when setting rich text" do
|
||||||
|
it 'escapes any HTML special characters in the diff chunk header' do
|
||||||
|
subject = described_class.new("<input>", "", 0, 0, 0)
|
||||||
|
line = subject.as_json
|
||||||
|
|
||||||
|
expect(line[:rich_text]).to eq("<input>")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in New Issue