Commit a6cce3a6 authored by Alex Kalderimis's avatar Alex Kalderimis Committed by GitLab Release Tools Bot

Prevent DOS via excessive ASCIIDOC includes

This sets a maximum limit on the number of include directives that any
one file may use (transitively). This is set relatively low at the
moment to 32 since each one requires a gitlay call to retrieve a blob
from the repo, and experimentation suggests that this keeps processing
to under 10sec.

This approach prevents both cyclic imports as well as having a single
file with an extremely lang list of includes.
parent 7402bea1
---
title: Limit number of AsciiDoc includes per document
merge_request:
author:
type: security
......@@ -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
......
......@@ -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
......
......@@ -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
......
# 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 'allows the Nth + 1 include' do
(max_includes - 1).times { processor.send(:read_blob, ref, 'a.adoc') }
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
......@@ -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' }
......
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