diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-28 15:38:36 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-28 15:38:36 +0000 |
commit | 24d41a30c09ecd7dcb2938d89fdeacb392606913 (patch) | |
tree | 8234b1d267155e9d58730a4014a842ece265b522 | |
parent | 6f830564667f76801e322567957f2a84c8158bbf (diff) | |
download | gitlab-ce-24d41a30c09ecd7dcb2938d89fdeacb392606913.tar.gz |
Add latest changes from gitlab-org/security/gitlab@12-6-stable-ee
-rw-r--r-- | changelogs/unreleased/security-dos-via-asciidoc-includes.yml | 5 | ||||
-rw-r--r-- | doc/user/asciidoc.md | 5 | ||||
-rw-r--r-- | lib/gitlab/asciidoc.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/asciidoc/include_processor.rb | 13 | ||||
-rw-r--r-- | spec/lib/gitlab/asciidoc/include_processor_spec.rb | 44 | ||||
-rw-r--r-- | spec/lib/gitlab/asciidoc_spec.rb | 18 |
6 files changed, 84 insertions, 3 deletions
diff --git a/changelogs/unreleased/security-dos-via-asciidoc-includes.yml b/changelogs/unreleased/security-dos-via-asciidoc-includes.yml new file mode 100644 index 00000000000..8fc3bd32316 --- /dev/null +++ b/changelogs/unreleased/security-dos-via-asciidoc-includes.yml @@ -0,0 +1,5 @@ +--- +title: Limit number of AsciiDoc includes per document +merge_request: +author: +type: security diff --git a/doc/user/asciidoc.md b/doc/user/asciidoc.md index b4d3cb58e97..da6bf287955 100644 --- a/doc/user/asciidoc.md +++ b/doc/user/asciidoc.md @@ -221,6 +221,11 @@ include::basics.adoc[] include::https://example.org/installation.adoc[] ``` +To guarantee good system performance and prevent malicious documents causing +problems, GitLab enforces a **maximum limit** on the number of include directives +processed in any one document. Currently a total of 32 documents can be +included, a number that is inclusive of transitive dependencies. + ### Blocks ```asciidoc diff --git a/lib/gitlab/asciidoc.rb b/lib/gitlab/asciidoc.rb index da65caa6c9c..8d072422e17 100644 --- a/lib/gitlab/asciidoc.rb +++ b/lib/gitlab/asciidoc.rb @@ -11,6 +11,7 @@ module Gitlab # the resulting HTML through HTML pipeline filters. module Asciidoc MAX_INCLUDE_DEPTH = 5 + MAX_INCLUDES = 32 DEFAULT_ADOC_ATTRS = { 'showtitle' => true, 'sectanchors' => true, @@ -40,6 +41,7 @@ module Gitlab extensions: extensions } context[:pipeline] = :ascii_doc + context[:max_includes] = [MAX_INCLUDES, context[:max_includes]].compact.min plantuml_setup diff --git a/lib/gitlab/asciidoc/include_processor.rb b/lib/gitlab/asciidoc/include_processor.rb index c6fbf540e9c..c3dca21b4b5 100644 --- a/lib/gitlab/asciidoc/include_processor.rb +++ b/lib/gitlab/asciidoc/include_processor.rb @@ -13,7 +13,9 @@ module Gitlab super(logger: Gitlab::AppLogger) @context = context - @repository = context[:project].try(:repository) + @repository = context[:repository] || context[:project].try(:repository) + @max_includes = context[:max_includes].to_i + @included = [] # Note: Asciidoctor calls #freeze on extensions, so we can't set new # instance variables after initialization. @@ -28,8 +30,11 @@ module Gitlab def include_allowed?(target, reader) doc = reader.document - return false if doc.attributes.fetch('max-include-depth').to_i < 1 + max_include_depth = doc.attributes.fetch('max-include-depth').to_i + + return false if max_include_depth < 1 return false if target_uri?(target) + return false if included.size >= max_includes true end @@ -62,7 +67,7 @@ module Gitlab private - attr_accessor :context, :repository, :cache + attr_reader :context, :repository, :cache, :max_includes, :included # Gets a Blob at a path for a specific revision. # This method will check that the Blob exists and contains readable text. @@ -77,6 +82,8 @@ module Gitlab raise 'Blob not found' unless blob raise 'File is not readable' unless blob.readable_text? + included << filename + blob end diff --git a/spec/lib/gitlab/asciidoc/include_processor_spec.rb b/spec/lib/gitlab/asciidoc/include_processor_spec.rb new file mode 100644 index 00000000000..5fec4d9e208 --- /dev/null +++ b/spec/lib/gitlab/asciidoc/include_processor_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'nokogiri' + +describe Gitlab::Asciidoc::IncludeProcessor do + let_it_be(:project) { create(:project, :repository) } + + let(:processor_context) do + { + project: project, + max_includes: max_includes, + ref: ref + } + end + let(:ref) { project.repository.root_ref } + let(:max_includes) { 10 } + + let(:reader) { Asciidoctor::PreprocessorReader.new(document, lines, 'file.adoc') } + let(:document) { Asciidoctor::Document.new(lines) } + + subject(:processor) { described_class.new(processor_context) } + + let(:a_blob) { double(:Blob, readable_text?: true, data: a_data) } + let(:a_data) { StringIO.new('include::b.adoc[]') } + + let(:lines) { [':max-include-depth: 1000'] + Array.new(10, 'include::a.adoc[]') } + + before do + allow(project.repository).to receive(:blob_at).with(ref, 'a.adoc').and_return(a_blob) + end + + describe '#include_allowed?' do + it 'allows the first include' do + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_truthy + end + + it 'disallows the Nth + 1 include' do + max_includes.times { processor.send(:read_blob, ref, 'a.adoc') } + + expect(processor.send(:include_allowed?, 'foo.adoc', reader)).to be_falsey + end + end +end diff --git a/spec/lib/gitlab/asciidoc_spec.rb b/spec/lib/gitlab/asciidoc_spec.rb index 38ec04ebe81..e962951e6d4 100644 --- a/spec/lib/gitlab/asciidoc_spec.rb +++ b/spec/lib/gitlab/asciidoc_spec.rb @@ -425,6 +425,24 @@ module Gitlab create_file(current_file, "= AsciiDoc\n") end + def many_includes(target) + Array.new(10, "include::#{target}[]").join("\n") + end + + context 'cyclic imports' do + before do + create_file('doc/api/a.adoc', many_includes('b.adoc')) + create_file('doc/api/b.adoc', many_includes('a.adoc')) + end + + let(:include_path) { 'a.adoc' } + let(:requested_path) { 'doc/api/README.md' } + + it 'completes successfully' do + is_expected.to include('<p>Include this:</p>') + end + end + context 'with path to non-existing file' do let(:include_path) { 'not-exists.adoc' } |