Commit 17015e66 authored by Patrick Bajao's avatar Patrick Bajao

Sanitize output by dependency linkers

When there are URLs defined in some dependency file (e.g. Gemfile,
gemspec, etc), they get converted to links. We are not sanitizing
it so if some `javascript:` code is added as a URL, it can cause
XSS vulnerability.
parent edece6f7
---
title: Sanitize output by dependency linkers
merge_request:
author:
type: security
...@@ -7,6 +7,8 @@ module Gitlab ...@@ -7,6 +7,8 @@ module Gitlab
GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze GIT_INVALID_URL_REGEX = /^git\+#{URL_REGEX}/.freeze
REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze REPO_REGEX = %r{[^/'" ]+/[^/'" ]+}.freeze
include ActionView::Helpers::SanitizeHelper
class_attribute :file_type class_attribute :file_type
def self.support?(blob_name) def self.support?(blob_name)
...@@ -62,7 +64,10 @@ module Gitlab ...@@ -62,7 +64,10 @@ module Gitlab
end end
def link_tag(name, url) def link_tag(name, url)
%{<a href="#{ERB::Util.html_escape_once(url)}" rel="nofollow noreferrer noopener" target="_blank">#{ERB::Util.html_escape_once(name)}</a>}.html_safe sanitize(
%{<a href="#{ERB::Util.html_escape_once(url)}" rel="nofollow noreferrer noopener" target="_blank">#{ERB::Util.html_escape_once(name)}</a>},
attributes: %w[href rel target]
)
end end
# Links package names based on regex. # Links package names based on regex.
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::DependencyLinker::BaseLinker do
let(:linker_class) do
Class.new(described_class) do
def link_dependencies
link_regex(%r{^(?<name>https?://[^ ]+)}, &:itself)
end
end
end
let(:plain_content) do
<<~CONTENT
http://\\njavascript:alert(1)
https://gitlab.com/gitlab-org/gitlab
CONTENT
end
let(:highlighted_content) do
<<~CONTENT
<span><span>http://</span><span>\\n</span><span>javascript:alert(1)</span></span>
<span><span>https://gitlab.com/gitlab-org/gitlab</span></span>
CONTENT
end
let(:linker) { linker_class.new(plain_content, highlighted_content) }
describe '#link' do
subject { linker.link }
it 'only converts valid links' do
expect(subject).to eq(
<<~CONTENT
<span><span>#{link('http://')}</span><span>#{link('\n', url: '%5Cn')}</span><span>#{link('javascript:alert(1)', url: nil)}</span></span>
<span><span>#{link('https://gitlab.com/gitlab-org/gitlab')}</span></span>
CONTENT
)
end
end
def link(text, url: text)
attrs = [
'rel="nofollow noreferrer noopener"',
'target="_blank"'
]
attrs.unshift(%{href="#{url}"}) if url
%{<a #{attrs.join(' ')}>#{text}</a>}
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