From dcae7fab92a93f3750831b4e70e9b61d3c064b83 Mon Sep 17 00:00:00 2001
From: Alexandru Croitor <acroitor@gitlab.com>
Date: Wed, 17 Jul 2019 12:54:40 +0300
Subject: Limit the size of issuable description and comments

Limiting the size of issuable description and comments to 1_000_000,
which is close to ~1MB of ASCII characters, which represents 99.9% of
all descriptions and comments we have in DB at the moment. This should
help prevent DoS attacks when comments contain refference strings.

Also this change updates regexp matching the namespaces paths by
limiting the namespaces paths to Namespace::NUMBER_OF_ANCESTORS_ALLOWED,
as we allow 20 levels deep groups.

see https://gitlab.com/gitlab-org/gitlab-ce/issues/61974#note_191274234
---
 .../banzai/filter/project_reference_filter_spec.rb | 16 +++++--
 spec/models/concerns/issuable_spec.rb              | 51 ++++++++++++++++++++++
 spec/models/note_spec.rb                           |  1 +
 .../models/concern/issuable_shared_examples.rb     |  8 ++++
 4 files changed, 72 insertions(+), 4 deletions(-)
 create mode 100644 spec/support/shared_examples/models/concern/issuable_shared_examples.rb

(limited to 'spec')

diff --git a/spec/lib/banzai/filter/project_reference_filter_spec.rb b/spec/lib/banzai/filter/project_reference_filter_spec.rb
index 69f9c1ae829..927d226c400 100644
--- a/spec/lib/banzai/filter/project_reference_filter_spec.rb
+++ b/spec/lib/banzai/filter/project_reference_filter_spec.rb
@@ -26,10 +26,18 @@ describe Banzai::Filter::ProjectReferenceFilter do
     expect(reference_filter(act).to_html).to eq(CGI.escapeHTML(exp))
   end
 
-  it 'fails fast for long invalid string' do
-    expect do
-      Timeout.timeout(5.seconds) { reference_filter("A" * 50000).to_html }
-    end.not_to raise_error
+  context 'when invalid reference strings are very long' do
+    shared_examples_for 'fails fast' do |ref_string|
+      it 'fails fast for long strings' do
+        # took well under 1 second in CI https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/3267#note_172824
+        expect do
+          Timeout.timeout(3.seconds) { reference_filter(ref_string).to_html }
+        end.not_to raise_error
+      end
+    end
+
+    it_behaves_like 'fails fast', 'A' * 50000
+    it_behaves_like 'fails fast', '/a' * 50000
   end
 
   it 'allows references with text after the > character' do
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 68224a56515..537bad98fa4 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -46,6 +46,7 @@ describe Issuable do
       it { is_expected.to validate_presence_of(:author) }
       it { is_expected.to validate_presence_of(:title) }
       it { is_expected.to validate_length_of(:title).is_at_most(255) }
+      it { is_expected.to validate_length_of(:description).is_at_most(1_000_000) }
     end
 
     describe 'milestone' do
@@ -774,4 +775,54 @@ describe Issuable do
       end
     end
   end
+
+  describe '#matches_cross_reference_regex?' do
+    context "issue description with long path string" do
+      let(:mentionable) { build(:issue, description: "/a" * 50000) }
+
+      it_behaves_like 'matches_cross_reference_regex? fails fast'
+    end
+
+    context "note with long path string" do
+      let(:mentionable) { build(:note, note: "/a" * 50000) }
+
+      it_behaves_like 'matches_cross_reference_regex? fails fast'
+    end
+
+    context "note with long path string" do
+      let(:project) { create(:project, :public, :repository) }
+      let(:mentionable) { project.commit }
+
+      before do
+        expect(mentionable.raw).to receive(:message).and_return("/a" * 50000)
+      end
+
+      it_behaves_like 'matches_cross_reference_regex? fails fast'
+    end
+  end
+
+  describe '#matches_cross_reference_regex?' do
+    context "issue description with long path string" do
+      let(:mentionable) { build(:issue, description: "/a" * 50000) }
+
+      it_behaves_like 'matches_cross_reference_regex? fails fast'
+    end
+
+    context "note with long path string" do
+      let(:mentionable) { build(:note, note: "/a" * 50000) }
+
+      it_behaves_like 'matches_cross_reference_regex? fails fast'
+    end
+
+    context "note with long path string" do
+      let(:project) { create(:project, :public, :repository) }
+      let(:mentionable) { project.commit }
+
+      before do
+        expect(mentionable.raw).to receive(:message).and_return("/a" * 50000)
+      end
+
+      it_behaves_like 'matches_cross_reference_regex? fails fast'
+    end
+  end
 end
diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb
index 03003e3dd7d..5bc0673dd73 100644
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -22,6 +22,7 @@ describe Note do
   end
 
   describe 'validation' do
+    it { is_expected.to validate_length_of(:note).is_at_most(1_000_000) }
     it { is_expected.to validate_presence_of(:note) }
     it { is_expected.to validate_presence_of(:project) }
 
diff --git a/spec/support/shared_examples/models/concern/issuable_shared_examples.rb b/spec/support/shared_examples/models/concern/issuable_shared_examples.rb
new file mode 100644
index 00000000000..9604555c57d
--- /dev/null
+++ b/spec/support/shared_examples/models/concern/issuable_shared_examples.rb
@@ -0,0 +1,8 @@
+shared_examples_for 'matches_cross_reference_regex? fails fast' do
+  it 'fails fast for long strings' do
+    # took well under 1 second in CI https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/3267#note_172823
+    expect do
+      Timeout.timeout(3.seconds) { mentionable.matches_cross_reference_regex? }
+    end.not_to raise_error
+  end
+end
-- 
cgit v1.2.1