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

Logic bug regarding spinner-based protection #133

Closed
se2342 opened this issue Mar 10, 2024 · 4 comments · Fixed by #134
Closed

Logic bug regarding spinner-based protection #133

se2342 opened this issue Mar 10, 2024 · 4 comments · Fixed by #134

Comments

@se2342
Copy link

se2342 commented Mar 10, 2024

Expected Behaviour

When setting spinner_enabled = true requests with an empty params[:spinner] should fail regardless of whether the client has requested a view that invokes invisible_captcha form helper.

Actual Behaviour

When spinner_enabled = true is set but no previous Rails view has been called, session[:invisible_captcha_spinner] is empty thus not making valid params[:spinner] a prerequisite for the request to be executed i.e. malicious requests are not detected as spam, effectively bypassing the spinner-based protection.

See

if InvisibleCaptcha.spinner_enabled && params[:spinner] != session[:invisible_captcha_spinner]

Proposal

Improve conditional logic as follows:

if InvisibleCaptcha.spinner_enabled && (params[:spinner].blank? || params[:spinner] != session[:invisible_captcha_spinner])
@markets
Copy link
Owner

markets commented Mar 10, 2024

Hello @se2342,

I think that makes sense, I can't remember now a valid reason to not have this extra check in place.

@markets
Copy link
Owner

markets commented Mar 17, 2024

Hello again @se2342,

I just pushed this change in #134. I'll cut a new release to RubyGems soon: v2.2.0...master

@markets
Copy link
Owner

markets commented Mar 17, 2024

UPDATE I finally released a new version now 🚀 https://rubygems.org/gems/invisible_captcha/versions/2.3.0

@se2342
Copy link
Author

se2342 commented Mar 17, 2024

Awesome. Thank you for your work as a maintainer. 👏

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