Commit d4e54b7e authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'security-fj-stored-xss-in-repository-imports' into 'master'

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

See merge request gitlab/gitlabhq!2492
parents b8cf41bc c40400ce
...@@ -9,6 +9,6 @@ class DiffLineEntity < Grape::Entity ...@@ -9,6 +9,6 @@ class DiffLineEntity < Grape::Entity
expose :meta_positions, as: :meta_data expose :meta_positions, as: :meta_data
expose :rich_text do |line| expose :rich_text do |line|
line.rich_text || CGI.escapeHTML(line.text) ERB::Util.html_escape(line.rich_text || line.text)
end end
end end
---
title: Fix stored XSS in merge requests from imported repository
merge_request:
author:
type: security
...@@ -24,7 +24,7 @@ module Gitlab ...@@ -24,7 +24,7 @@ module Gitlab
# ignore highlighting for "match" lines # ignore highlighting for "match" lines
next diff_line if diff_line.meta? 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] if line_inline_diffs = inline_diffs[i]
begin begin
......
...@@ -8,6 +8,20 @@ describe Gitlab::Diff::Highlight do ...@@ -8,6 +8,20 @@ describe Gitlab::Diff::Highlight do
let(:diff) { commit.raw_diffs.first } let(:diff) { commit.raw_diffs.first }
let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: commit.diff_refs, repository: project.repository) } 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 describe '#highlight' do
context "with a diff file" do context "with a diff file" do
let(:subject) { described_class.new(diff_file, repository: project.repository).highlight } let(:subject) { described_class.new(diff_file, repository: project.repository).highlight }
...@@ -38,6 +52,16 @@ describe Gitlab::Diff::Highlight do ...@@ -38,6 +52,16 @@ describe Gitlab::Diff::Highlight do
expect(subject[5].rich_text).to eq(code) expect(subject[5].rich_text).to eq(code)
end 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 end
context "with diff lines" do context "with diff lines" do
...@@ -93,6 +117,10 @@ describe Gitlab::Diff::Highlight do ...@@ -93,6 +117,10 @@ describe Gitlab::Diff::Highlight do
expect { subject }. to raise_exception(RangeError) expect { subject }. to raise_exception(RangeError)
end end
end end
context 'when no inline diffs' do
it_behaves_like 'without inline diffs'
end
end end
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