search

alxy / oc-captcha-plugin

Integrates Googles reCAPTCHA into October.
14 stars 7 forks source link

Potential design flaw #5

Open codee opened 7 years ago

codee commented 7 years ago

Correct me if I'm wrong but the check for 'g-recaptcha-response' in CaptchaMiddleware means that the captcha can be bypassed altogether simply by not including it in the request (e.g., by removing the captcha element from the DOM.)

alxy commented 7 years ago

This appears to be true, however, I don't know how we could solve this nicely. It would require us to somehow else tell the middleware when it should check. Any ideas?

codee commented 7 years ago

I'm not sure this problem can be solved using a middleware. In our own captcha plugin my colleague implemented a custom validation rule for the captcha which seems to be clean enough for our purposes.

mauserrifle commented 6 years ago

This is indeed a serious issue and makes the plugin useless really. I put a project live with this plugin and received major spam in the first day. I also made a quickfix using validation. When I got time I might make a pull request with a proper fix :)

viamage commented 6 years ago

@mauserrifle could you find some time for PR or describe your fix idea?

mohsin commented 5 years ago

@alxy You can solve this nicely using the following process (it's complex but solves the issue):

  1. Every time a user adds a captcha component somewhere it needs to register itself in an array of forms with captchas i.e. collect the list of where the captcha is being used with the intended actions i.e. either forms or October's request actions. This collection of captcha form logic is similar to how the Rainlab's translate plugin scans for strings that are to be translated.
  2. In your middleware layer, rather than checking for the request as to whether it contains g-recaptcha-response instead check if the request exists in this array of form requests (I think you will need to have 2 parameters one is the type of request i.e. OctoberCMS Ajax framework request or a regular POST request) and then it's value which could be something like "onSubmitForm" or "http://mywebsite.com/submit_form" respectively.
  3. So now you are able to filter down only requests that have the captcha in them. The rest of the logic is the same.

Since the form action collection logic is complex to build, I would actually go about giving a manual entry system where developers can enter the type of request and it's value into a list and then this list can be used to filter down requests in the middleware. This would fix the plugin and render it useful again. After that, I would figure out the best way to collect the POST actions dynamically.

alxy commented 5 years ago

@SaifurRahmanMohsin Yeah, something like this would certainly work. Would you be able to come up with a PR for this?

mohsin commented 5 years ago

Hey @alxy, I do not have the time to work on this at the moment. I've got a hectic month ahead with enough client work and personal work so I've kept FOSS work on hold for now. I would suggest you do this on your own.