Fix color validation regex

Also prevents ReDoS vulnerability

3 files changed, 49 insertions, 1 deletions

diff --git a/app/validators/color_validator.rb b/app/validators/color_validator.rb
index 1932d042e83..974dfbbf394 100644
--- a/app/validators/color_validator.rb
+++ b/app/validators/color_validator.rb
@@ -12,7 +12,7 @@
 #   end
 #
 class ColorValidator < ActiveModel::EachValidator
-  PATTERN = /\A\#[0-9A-Fa-f]{3}{1,2}+\Z/.freeze
+  PATTERN = /\A\#(?:[0-9A-Fa-f]{3}){1,2}\Z/.freeze
 
   def validate_each(record, attribute, value)
     unless value =~ PATTERN
diff --git a/changelogs/unreleased/security-2858-fix-color-validation.yml b/changelogs/unreleased/security-2858-fix-color-validation.yml
new file mode 100644
index 00000000000..3430207a2b6
--- /dev/null
+++ b/changelogs/unreleased/security-2858-fix-color-validation.yml
@@ -0,0 +1,5 @@
+---
+title: Fix DoS vulnerability in color validation regex
+merge_request:
+author:
+type: security
diff --git a/spec/validators/color_validator_spec.rb b/spec/validators/color_validator_spec.rb
new file mode 100644
index 00000000000..e5a38ac9372
--- /dev/null
+++ b/spec/validators/color_validator_spec.rb
@@ -0,0 +1,43 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ColorValidator do
+  using RSpec::Parameterized::TableSyntax
+
+  subject do
+    Class.new do
+      include ActiveModel::Model
+      include ActiveModel::Validations
+      attr_accessor :color
+      validates :color, color: true
+    end.new
+  end
+
+  where(:color, :is_valid) do
+    '#000abc'    | true
+    '#aaa'       | true
+    '#BBB'       | true
+    '#cCc'       | true
+    '#ffff'      | false
+    '#000111222' | false
+    'invalid'    | false
+    '000'        | false
+  end
+
+  with_them do
+    it 'only accepts valid colors' do
+      subject.color = color
+
+      expect(subject.valid?).to eq(is_valid)
+    end
+  end
+
+  it 'fails fast for long invalid string' do
+    subject.color = '#' + ('0' * 50_000) + 'xxx'
+
+    expect do
+      Timeout.timeout(5.seconds) { subject.valid? }
+    end.not_to raise_error
+  end
+end