Commit c40400ce authored by Francisco Javier López's avatar Francisco Javier López Committed by Bob Van Landuyt

[master] Stored XSS in Gitlab Merge Request from imported repository

parent b8cf41bc
......@@ -9,6 +9,6 @@ class DiffLineEntity < Grape::Entity
expose :meta_positions, as: :meta_data
expose :rich_text do |line|
line.rich_text || CGI.escapeHTML(line.text)
ERB::Util.html_escape(line.rich_text || line.text)
end
end
---
title: Fix stored XSS in merge requests from imported repository
merge_request:
author:
type: security
......@@ -24,7 +24,7 @@ def highlight
# ignore highlighting for "match" lines
next diff_line if diff_line.meta?
rich_line = highlight_line(diff_line) || diff_line.text
rich_line = highlight_line(diff_line) || ERB::Util.html_escape(diff_line.text)
if line_inline_diffs = inline_diffs[i]
begin
......
......@@ -8,6 +8,20 @@
let(:diff) { commit.raw_diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) }
shared_examples 'without inline diffs' do
let(:code) { '<h2 onmouseover="alert(2)">Test</h2>' }
before do
allow(Gitlab::Diff::InlineDiff).to receive(:for_lines).and_return([])
allow_any_instance_of(Gitlab::Diff::Line).to receive(:text).and_return(code)
end
it 'returns html escaped diff text' do
expect(subject[1].rich_text).to eq html_escape(code)
expect(subject[1].rich_text).to be_html_safe
end
end
describe '#highlight' do
context "with a diff file" do
let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
......@@ -38,6 +52,16 @@
expect(subject[5].rich_text).to eq(code)
end
context 'when no diff_refs' do
before do
allow(diff_file).to receive(:diff_refs).and_return(nil)
end
context 'when no inline diffs' do
it_behaves_like 'without inline diffs'
end
end
end
context "with diff lines" do
......@@ -93,6 +117,10 @@
expect { subject }. to raise_exception(RangeError)
end
end
context 'when no inline diffs' do
it_behaves_like 'without inline diffs'
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe DiffLineEntity do
include RepoHelpers
let(:code) { 'hello world' }
let(:line) { Gitlab::Diff::Line.new(code, 'new', 1, nil, 1) }
let(:entity) { described_class.new(line, request: {}) }
subject { entity.as_json }
it 'exposes correct attributes' do
expect(subject).to include(
:line_code, :type, :old_line, :new_line, :text, :meta_data, :rich_text
)
end
describe '#rich_text' do
let(:code) { '<h2 onmouseover="alert(2)">Test</h2>' }
let(:rich_text_value) { nil }
before do
line.instance_variable_set(:@rich_text, rich_text_value)
end
shared_examples 'escapes html tags' do
it do
expect(subject[:rich_text]).to eq html_escape(code)
expect(subject[:rich_text]).to be_html_safe
end
end
context 'when rich_line is present' do
let(:rich_text_value) { code }
it_behaves_like 'escapes html tags'
end
context 'when rich_line is not present' do
it_behaves_like 'escapes html tags'
end
end
end
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