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 | 110592dfad951d3a3ad8d1ea6a12e8a2dbc3815a (patch) | |
tree | ae0224a63d1fc55541625e769942b78395b5daaf | |
parent | 3e5f4a8802d60908fe3bcf56a72afd15e1a4777a (diff) | |
download | gitlab-ce-110592dfad951d3a3ad8d1ea6a12e8a2dbc3815a.tar.gz |
Add latest changes from gitlab-org/security/gitlab@12-7-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 | 11 | ||||
-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, 83 insertions, 2 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 6e0b7ce60ba..53d1135a2d7 100644 --- a/lib/gitlab/asciidoc/include_processor.rb +++ b/lib/gitlab/asciidoc/include_processor.rb @@ -14,6 +14,8 @@ module Gitlab @context = context @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 c8d159d1e84..c7156a500d0 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' } |