Fix color validation regex
Also prevents ReDoS vulnerability
This commit is contained in:
parent
db9783f782
commit
717824144f
3 changed files with 49 additions and 1 deletions
|
@ -12,7 +12,7 @@
|
||||||
# end
|
# end
|
||||||
#
|
#
|
||||||
class ColorValidator < ActiveModel::EachValidator
|
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)
|
def validate_each(record, attribute, value)
|
||||||
unless value =~ PATTERN
|
unless value =~ PATTERN
|
||||||
|
|
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Fix DoS vulnerability in color validation regex
|
||||||
|
merge_request:
|
||||||
|
author:
|
||||||
|
type: security
|
43
spec/validators/color_validator_spec.rb
Normal file
43
spec/validators/color_validator_spec.rb
Normal file
|
@ -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
|
Loading…
Reference in a new issue