Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReDoS vulnerability in ActiveAttr::Typecasting::BooleanTypecaster#call #184

Closed
wonda-tea-coffee opened this issue Mar 14, 2021 · 2 comments · Fixed by #185
Closed

ReDoS vulnerability in ActiveAttr::Typecasting::BooleanTypecaster#call #184

wonda-tea-coffee opened this issue Mar 14, 2021 · 2 comments · Fixed by #185

Comments

@wonda-tea-coffee
Copy link
Contributor

wonda-tea-coffee commented Mar 14, 2021

Detail

This method determines if it matches the following regular expression.
https://github.com/cgriego/active_attr/blob/v0.15.2/lib/active_attr/typecasting/boolean_typecaster.rb#L45

For example, it will take a lot of time to match if the input is something like this.

"#{?0 * 100000}a" =~ /\A[-+]?(0+\.?0*|0*\.?0+)\z/

Solution

Use possessive quantifier.
This will return an immediate result for the input in question, while maintaining compatibility.

diff --git a/lib/active_attr/typecasting/boolean_typecaster.rb b/lib/active_attr/typecasting/boolean_typecaster.rb
index 8b84102..5250ca8 100644
--- a/lib/active_attr/typecasting/boolean_typecaster.rb
+++ b/lib/active_attr/typecasting/boolean_typecaster.rb
@@ -42,7 +42,7 @@ module ActiveAttr
         case value
         when *FALSE_VALUES then false
         when *NIL_VALUES then nil
- when Numeric, /\A[-+]?(0+\.?0*|0*\.?0+)\z/ then !value.to_f.zero?
+ when Numeric, /\A[-+]?(0++\.?0*|0*+\.?0+)\z/ then !value.to_f.zero?
         else value.present?
         end
       end
@wonda-tea-coffee wonda-tea-coffee changed the title ReDoS Vulnerability in ActiveAttr::Typecasting::BooleanTypecaster#call ReDoS vulnerability in ActiveAttr::Typecasting::BooleanTypecaster#call Mar 14, 2021
@cgriego
Copy link
Owner

cgriego commented Apr 8, 2021

Sounds good to me. Can you open a pull request? @wonda-tea-coffee

@wonda-tea-coffee
Copy link
Contributor Author

wonda-tea-coffee commented Apr 8, 2021

@cgriego
I made a PR 👍
#185

cgriego pushed a commit that referenced this issue Apr 12, 2021
resolve #184

Co-authored-by: wonda-tea-coffee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants