Allow extended regexp into validate format #885

Merged
merged 2 commits into from May 26, 2016

Projects

None yet

2 participants

@c0va23
Contributor
c0va23 commented May 24, 2016

Hi!

I just want to apologize for my English. I use Google Translate for this message.

I add support of extended regexes into checking of validate format.

I like to write complex regular expressions in a multiline format, but the brakeman displays false warnings on these regular expressions.

In tests I've added an example of such a regular expression.

@presidentbeef presidentbeef commented on an outdated diff May 24, 2016
lib/brakeman/checks/check_validation_regex.rb
@@ -59,17 +59,37 @@ def process_validates validator
end
end
+ # Match secure regexp without extended option
+ SECURE_REGEXP_PATTER = %r{
+ \A
+ \\A
+ .*
+ \\[z|Z]
@presidentbeef
presidentbeef May 24, 2016 Owner

This is not the same as (z|Z). User either (z|Z) or [zZ]:

2.3.0 :001 > "|".match /[z|Z]/
 => #<MatchData "|">
2.3.0 :002 > "|".match(/(z|Z)/)
 => nil
@presidentbeef presidentbeef commented on an outdated diff May 24, 2016
lib/brakeman/checks/check_validation_regex.rb
@@ -59,17 +59,37 @@ def process_validates validator
end
end
+ # Match secure regexp without extended option
+ SECURE_REGEXP_PATTER = %r{
@presidentbeef
presidentbeef May 24, 2016 Owner

Please add the N on the end of this constant and the other one.

@presidentbeef presidentbeef commented on an outdated diff May 24, 2016
lib/brakeman/checks/check_validation_regex.rb
@@ -59,17 +59,37 @@ def process_validates validator
end
end
+ # Match secure regexp without extended option
+ SECURE_REGEXP_PATTER = %r{
+ \A
+ \\A
+ .*
+ \\[z|Z]
+ \z
+ }x
+j
@presidentbeef
presidentbeef May 24, 2016 Owner

What is this j doing here and how are tests passing with it...

@presidentbeef presidentbeef commented on an outdated diff May 24, 2016
test/tests/rails_with_xss_plugin.rb
@@ -299,6 +299,15 @@ def test_format_validation_21
:file => /user\.rb/
end
+ def test_format_validation_with_multiline
@presidentbeef
presidentbeef May 24, 2016 Owner

This should be in test/tests/rails4.rb not here

@c0va23
Contributor
c0va23 commented May 24, 2016

@presidentbeef, i fix all tupos and notes

@presidentbeef presidentbeef merged commit fcf8cf7 into presidentbeef:master May 26, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@presidentbeef
Owner

Hi Fedorenko,

Thank you for your contribution and making those changes!

@presidentbeef presidentbeef locked and limited conversation to collaborators Oct 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.