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

Minimum length validation for the recaptcha token param #462

Merged
merged 10 commits into from
Jan 14, 2025

Conversation

Akashkarmakar787
Copy link
Contributor

Add minimum length validation for the recaptcha token param

Reason:
#461

Spammers can pass random value for the token param (ex: g-recaptcha-response) which can increase the API validation call to recaptcha, consequently incurring more cost.

Most of the Spam attacks we observed attacher sending 1-10 length chars.
It is good to have minimum length 100, validation check to avoid unnecessary API calls to recaptcha.

@@ -588,7 +588,7 @@ Recaptcha.configure do |config|
config.secret_key = '6Lc6BAAAAAAAAKN3DRm6VA_xxxxxxxxxxxxxxxxx'
config.verify_url = 'https://hcaptcha.com/siteverify'
config.api_server_url = 'https://hcaptcha.com/1/api.js'
config.response_limit = 100000
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would break anyone already using that field, how about adding response_minimum = 100 instead ?

@@ -180,7 +180,7 @@ def initialize
assert_equal "reCAPTCHA verification failed, please try again.", @controller.flash[:recaptcha_error]
end

it "does not verify via http call when response length exceeds G_RESPONSE_LIMIT" do
it "does not verify via http call when response length exceeds G_RESPONSE_MAX_LIMIT" do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think this should change, the name references the var we send to recapcha ... possibly should just be rewritten to "exceeds limit"

@grosser grosser merged commit eff11da into ambethia:master Jan 14, 2025
@grosser
Copy link
Collaborator

grosser commented Jan 14, 2025

🤦 ci was not working and this looked harmless :D
fixed in #464
5.19.0

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 this pull request may close these issues.

2 participants