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

Potential Polynomial regex used (says CodeQL) #18

Open
joshgoebel opened this issue Sep 25, 2024 · 2 comments
Open

Potential Polynomial regex used (says CodeQL) #18

joshgoebel opened this issue Sep 25, 2024 · 2 comments

Comments

@joshgoebel
Copy link

joshgoebel commented Sep 25, 2024

Due to #13 (and other version dependency issues) we've been forced to vendor a "fake" version 1.0 that fixes that issue within our project, but CodeQL isn't very happy about the regex being used.

Screenshot 2024-09-25 at 11 33 10 AM

This is exactly the same code present in this repo in: https://github.com/rack/rackup/blob/main/lib/rackup/handler.rb#L107-L108


I don't see the issue since [A-Z]+ and [^A-Z] have no overlap... does anyone else see it or is this a false positive?

@joshgoebel
Copy link
Author

joshgoebel commented Sep 25, 2024

Ruby 3.2.something:

# 100mb of repeated "A"s
> bad_input="/super/man/#{"A"*100000000}/lives"; nil
=> nil
[14] pry(main)> Benchmark.measure { bad_input.match(/[A-Z]+[^A-Z]/) }.real
=> 0.8092678760003764

Seems plenty fast to me...

@joshgoebel
Copy link
Author

Oh CodeQL did say the problem was much improved with Ruby 3.2, let me try 3.1...

>  bad_input="/super/man/#{"A"*100000000}/lives"; nil
=> nil
irb(main):007> RUBY_VERSION
=> "3.1.2"
irb(main):008>  Benchmark.measure { bad_input.match(/[A-Z]+[^A-Z]/); }.real
=> 0.4945120000047609
irb(main):009> bad_input.match(/[A-Z]+[^A-Z]/)[0].size
=> 100000001

Still seems fine.

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

No branches or pull requests

1 participant