diff options
author | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-06-12 22:48:38 +0800 |
---|---|---|
committer | Heinrich Lee Yu <heinrich@gitlab.com> | 2019-06-25 09:17:22 +0800 |
commit | 30b9b275522f2da80388e5f5a2187fe21d435c3a (patch) | |
tree | 03d3a66168586011fc40bc21985f5b16c788661a | |
parent | e3eeb779d72006b9fbbaecf9f1d8fbd52a7d6383 (diff) | |
download | gitlab-ce-30b9b275522f2da80388e5f5a2187fe21d435c3a.tar.gz |
Fix color validation regex
Also prevents ReDoS vulnerability
-rw-r--r-- | app/validators/color_validator.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/security-2858-fix-color-validation.yml | 5 | ||||
-rw-r--r-- | spec/validators/color_validator_spec.rb | 43 |
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 |