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