Hello,
After upgrading from 1.44 to 1.45, I've got the error during validating captcha field. Now the capctcha field can't be validating anymore.
The problem is in ajax request. It cannot be performed. Know the ajax url request (from http://example.com/clientside_validation/captcha) produeces 403 error.
Comments
Comment #2
alexey.r commentedComment #3
tombsage commentedHi alexey.r
I was having the same problem. Please try clearing all Drupal cache and also your browser cache and try again. This seems to have worked for me. Let me know if this also fixes the problem for you.
This seems to be happening when validating the token created for the CAPTCHA, line 229 of clientside_validation.module. The token is generated on line 1336 of clientside_validation.module.
Tom
Comment #4
alexey.r commentedUnfortunately after both cache cleaning the error still the same.
Comment #5
tombsage commentedI have just done the same thing on a different set of servers and I am still getting this problem.
line 1338 of clientside_validation.module - add the following:
watchdog('CAPTCHA-KEYS-1', json_encode(array(session_id(), drupal_get_private_key(), drupal_get_hash_salt())));
line 228 of clientside_validation.module - add the following:
watchdog('CAPTCHA-KEYS-2', json_encode(array(session_id(), drupal_get_private_key(), drupal_get_hash_salt())));
Clear cache and run through the captcha again. Then go to /admin/reports/dblog - if the session IDs are different then that is causing the problem.
Are you doing the captcha as an anonymous user?
Comment #6
joel_osc commentedSeeing this problem too... seems to work after second try in FF using private browsing window. But does not work in FF or Chrome in standard browsing window. All testing done as an anonymous drupal user.
Comment #7
tombsage commentedHi joel_osc,
Have you tried the method above to check if the session id's are different?
The drupal_valid_token() function does take a third parameter ($skip_anonymous: Set to true to skip token validation for anonymous users). This will make the captcha work but skips the token validation for anonymous users so I do not consider this a fix.
line 229 of clientside_validation.module:
What are your server setups? (e.g. load-balanced, single server?)
Comment #8
joel_osc commentedI put watchdog's on the lines above, but found that the second watchdog (CAPTCHA-KEYS-2) was only hit when the captcha was successful so I didn't have much to compare. I have it happening on a production site (simple single server) and in a lab environment server (even simpler) as well.
Comment #9
tombsage commentedThe second watchdog should be in the following position:
If you have it there, it should get hit even if the captcha is unsuccessful. (it's the "return drupal_access_denied();" line which returns the 403)
Comment #10
joel_osc commentedHi,
Thank-you for the outstanding help on this! Indeed you are correct the session ids are different in the two sections of code. The private key and hash salt are the same. I am not sure how the session ids could be different when all I am doing is loading the page (CAPTCHA-KEYS-1) and then answering the captcha (CAPTCHA-KEYS-2).
Comment #11
tombsage commentedHi,
I'm also not sure why the session is being re-generated. Do you have any info on the "Warning: Cannot modify header information - headers...Anonymous (not verified)" error? Perhaps it is related.
Comment #12
drupalevangelist commentedI encountered the similar issue today. I used the instructions from #5 to compare the session IDs and they are different. How should I go about resolving this issue? Any suggestions?
Comment #13
tombsage commentedHi emdadnrn,
You could turn off token validation for anon users (see #7) - again I'm not considering this an actual fix but it can get you up-and-running for now.
Comment #14
drupalevangelist commentedHi tombsage,
Thank you.
Comment #15
joel_osc commentedIn looking at this further I have added a bunch of watchdogs in the session.inc code and found that a session is never really created for anonymous users. And then looking at the API docs I found the following comment regarding drupal_get_token (https://api.drupal.org/api/drupal/includes!common.inc/function/drupal_ge...)
I not an expert in the area, so I would like to throw this out there for some thoughts...
Comment #16
nehasinghaniya21 commentedI am also having similar issue, where forms using clientside validation module showing error of incorrect captcha answer even though answer was correct.
I made changes as suggested by #7 (tombsage) and it works fine.
Thanks.
Comment #17
aaron.ferris commentedWill skipping the token validation for anonymous users reintroduce https://www.drupal.org/node/2907118?
Comment #18
alexey.r commented#7 receipt solved the problem.
Thank you.
Comment #19
tombsage commentedHi Aaron,
Skipping token validation for anon users does reintroduce this issue.
http://cgit.drupalcode.org/clientside_validation/commit/?id=b5b3ea2
https://www.drupal.org/node/2907118
#7 - "This will make the captcha work but skips the token validation for anonymous users so I do not consider this a fix."
joel_osc, you seem to on the right lines with manually starting a session. Perhaps Jelle_S could help with this?
Comment #20
aaron.ferris commentedHi @tombsage,
Thought as much.
I think the fix for this is to follow the guideline from drupal_get_token() and create a persistent session at the point of this token being created in _clientside_validation_set_captcha();
I'm working on a patch, ill attach it here when i've completed some testing.
Thanks
Aaron
Comment #21
tombsage commentedHi Aaron,
I've tried adding drupal_session_start(); to the start of the _clientside_validation_set_captcha() function. I am now getting matching session ids but the token validation is still failing.
Example:
_clientside_validation_set_captcha example vars ($js_rules[$name]['captcha']):
validate = 'captcha_validate_case_insensitive_ignore_spaces'
token = 'BkOwzzihjgS1mTkjoVPtZyznFztXSkjyc-zc8cameZg'
_clientside_validation_ajax_captcha() vars ($captcha_validate, $token)
$captcha_validate = 'captcha_validate_case_insensitive_ignore_spaces'
$token = 'BkOwzzihjgS1mTkjoVPtZyznFztXSkjyc-zc8cameZg'
Am I missing something? How is your testing going?
Comment #22
aaron.ferris commentedHi @tombsage
Testing on my patch seems to mitigate the 403 in my implementation, I'm just completing some further testing and ill attach it, if you could please apply it and see how you get on with it, that would be appreciated.
I'm just a little concerned over any potential performance impact of creating a session for anonymous users.
Thanks
Aaron
Comment #23
aaron.ferris commentedHere is the patch I'm working on.... as above, definitely concerns over any performance impact this could have.
TODO: If using this approach, we need to consider that the session would be destroyed as part of the ajax callback, but what if the user doesn't complete the form? We might need to look at similar destructive code elsewhere, perhaps hook_init() when the page request isn't 'clientside_validation/captcha'... needs some thought.
Edit: Bah, incorrect patch name :(
Thanks
Aaron
Comment #24
tombsage commentedHi Aaron,
I've applied your patch and I'm still getting a 403 from /clientside_validation/captcha. Does the session need to be started before we call drupal_get_token() in _clientside_validation_set_captcha()?
This would also mean if an anonymous user hits a page with a captcha but does not attempt it, their session will not be destroyed. This may impact caching back-ends such as Varnish for some setups.
EDIT: sorry didn't see the TODO
Comment #25
aaron.ferris commentedHi @tombsage
Seems to be fine for me in its current position, you could try moving it above to see if that helps?
My testing steps are:
1. Clear Drupal caches
2. Clear browser caches
3. Attempt to submit my Captcha enabled form
If I comment out drupal_session_start(); I start seeing the 403 again, so at least for me the fix seems to resolve my issue, quite why it's not working for you is strange though.
Out of interest, which version of the Captcha module are you using? I wonder if that could have an impact....
Thanks
Aaron
Comment #26
joel_osc commentedJust as an FYI, module like flag will use this module https://www.drupal.org/project/session_api to deal with anonymous users.
Comment #27
tombsage commentedHi Aaron,
I did have differing results when I first updated the module - worked fine (no patches) on dev server... 403 on live server. I have tried multiple browsers & cache clears and still get a 403 (using the patch).
I'm using captcha module 7.x-1.5.
Also _clientside_validation_ajax_captcha() could be called multiple times from AJAX requests per captcha. Do you actually want to destroy the session within this function?
Comment #28
aaron.ferris commentedHi @tombsage
Possibly not, the session destroy could come from elsewhere as noted in my 'TODO'. In fact if we're going to do that anyway, we could remove the destroy from _clientside_validation_ajax_captcha(). My patch was more to start a discussion on a possible fix, it's definitely not ready for use.
I'm not sure why we're seeing such differing results, perhaps I have something else in my codebase supporting this, hard to see what it could be mind. If the patch doesn't resolve your issue then it's clearly not fit for purpose for all use cases, hard for me to expand on this if I can't recreate it though :(.
Edit - Looking back at this thread, it seems your token is now matching during _clientside_validation_ajax_captcha()? Feels very strange why drupal_valid_token() would return false if the data matches.... is it possible the 403 is being caused elsewhere? My issue was that during _clientside_validation_ajax_captcha() the $token values were different which of course would cause a 403. The session_start mitigates this in my implementation.
@joel_osc could you please apply the patch attached and test? Or anyone else from this thread.
Perhaps a different approach to mitigating https://www.drupal.org/node/2907118 is needed, to move away from drupal_get_token if possible.
Edit - i'm also using Captcha 7.x-1.5
Thanks
Aaron
Comment #29
tombsage commentedHi Aaron,
I'm using image_captcha & image_captcha_refresh on webforms. Are you using a different challenge type?
After a bit more testing, it looks like the session being destroyed in _clientside_validation_ajax_captcha() is causing the 403. It seems to be calling _clientside_validation_ajax_captcha() multiple times while entering the captcha, destroying the session in _clientside_validation_ajax_captcha() is then making the sid change invalidating the token.
If we use hook_init to destroy the session, we will need to not destroy the session if the page being loaded contains a form with a captcha or is /clientside_validation/captcha - not sure how we would achieve this. Any ideas?
Comment #30
aaron.ferris commentedHi @tombsage
Indeed if this is firing multiple times then the subsequent calls to _clientside_validation_ajax_captcha() will have a fresh session ID reintroducing the 403, that makes sense. For reference i'm using a hidden captcha field from hidden_captcha on this particular form.
I've been playing with a method of destroying the session outside of this callback, and not getting very far. hook_init() isn't called on cached pages so I don't think we can go down this route. I've been attempting to do this in hook_boot() but I can't seem to tie down the correct code to ensure it isn't fired as part of 'clientside_validation/captcha'.....
My theory was to have something like:
But this seems to fire on every page call, including the AJAX callback, destroying the session and giving us the 403 again. I think we're on the right track with this, it's just a case of destroying the session in a reliable and correct manner.
I might look into destroying the session as part of a pre_process. Any ideas welcome on where else we could do this!
Thanks
Aaron
Comment #31
joel_osc commentedHi Aaron,
The patch fixes my site.
Cheers!
Comment #32
jelle_sThank you all for looking in to this. I currently don't have much time to spend on this. But I've been looking at the code in the captcha module. Captcha creates a sort of 'session' as well, and stores it in the database (the captcha_sessions table), where it also stores a token. If we could somehow create a token of the callback function and the captcha session id or token, in stead of using drupal_get_token, we could use that as well for preventing the arbitrary code execution mentioned in https://www.drupal.org/node/2907118
For those looking for a fix in the meantime, you can disable captcha validation (on the client side) in the config form of clientside validation. The captcha will still be validated on form submit by the captcha module, so no worries there.
Comment #33
aaron.ferris commentedThanks @joel_osc
Thanks @Jelle_s that makes sense and is definitely something I will try and look into to avoid drupal_get_token, also thanks for the heads up around disabling clientside_validation for Captcha as that's an option for the moment.
Aaron
Comment #34
aaron.ferris commentedHi @Jelle_s, @joel_osc, @tombsage,
Would something like the attached work (please ignore this patch and see the below comment)? It's essentially using the same approach as drupal_get_token() but substituting the session_id() for the captcha_token. This approach would pass the captcha_token through as a POST parameter as well, so I guess could also be open to manipulation but my thinking is for someone to manipulate this they'd need to know how we were generating the token in the first place? Feedback welcome.
I have tested this in my implementation and the 403 doesn't exist anymore, naturally my concern is around how robust this solution is to also avoid https://www.drupal.org/node/2907118.
I'll continue testing various scenarios, it would be good for others to please test this out if possible.
Edit: As a side note, it appears the documentation on https://www.drupal.org/node/2907118 is incorrect?
This seems to suggest the issue doesn't exist on 1.44, although the recommended solution is to install 1.45 this could still confuse some people, ill fire the security team a contact form.
Thanks
Aaron
Comment #35
aaron.ferris commentedIgnore that last patch, seemed I had a missing variable...
Comment #36
tombsage commentedHi Aaron,
Updated patch, would not apply as paths contained your full dev path. I will test this shortly.
Comment #37
aaron.ferris commentedThanks @tombsage, I had just realised this myself, so was coming on to attach a new patch!
Thanks
Aaron
Comment #38
aaron.ferris commentedCoding standards fix.
Comment #39
tombsage commentedHi Aaron
I forgot to mention the previous patch had the same problem. I have tested the latest patch and it seems to be working for me.
I believe an attacker would need to know the private key and salt to perform a successful attack, correct me if i'm wrong.
Hopefully a few more people can test the patch to cover all use cases.
Cheers
Tom
Comment #40
aaron.ferris commentedHi Tom,
That's my understanding as well, they'd need to know our method of creating the token as part of clientside_validation_generate_token() (including private key and salt) which is highly unlikely (perhaps even impossible?). Probably best to await @Jelle_s' input to be sure.
Thanks for the feedback.
Thanks
Aaron
Comment #41
jelle_sOverall the patch looks good to me. I'm just wondering if passing the captcha token to the frontend has any security implications (I would imagine not)... But if it has, we could pass the captcha session id and then get the captcha token from the database using that session id. Ideally we could have a maintainer of the captcha module chime in here, but I'm willing to commit the patch as-is as I don't really see a security issue here (I am not a security expert, but I don't think anyone can establish any sort of valuable information from that token).
I'll leave this issue open for a couple of days before I commit the patch, so that people have the chance to test this fix or to chime in.
Comment #42
tombsage commentedHi @Jelle_S
I tested using the captcha session id earlier and i'm not sure it would work as you could end up with multiple tokens per session id.
You could base it on the latest captcha token based on the session id but you could potentially have someone open multiple browser tabs with captcha's.
Comment #43
jelle_sHm that makes sense. As said, I'll leave it open for a couple of days to make sure, and to give people the chance to test. Feel free to remind me if I haven't committed this by monday.
Comment #44
tombsage commentedComment #45
aaron.ferris commentedI'm not sure exposing the Captcha token in the front end is a concern, if it is this is already happening by way of a hidden input field with the token value.
Comment #47
jelle_sFixed in 7.x-1.46
Comment #48
drupalevangelist commentedThank you. Installed the latest version 7.x-1.46 and the issue is permanently resolved.
Comment #49
aaron.ferris commentedThanks for the feedback @emdadnrn and @Jelle_S for the fast commit and new version release.
Guess this should be listed as fixed.
Thanks
Aaron
Comment #50
aaron.ferris commentedComment #51
jelle_s@aaron.ferris It's not yet fixed in 2.x :)
Comment #52
aaron.ferris commentedSure, thanks @Jelle_S.
I'll look at porting this when I get 5 minutes.
Comment #53
aaron.ferris commentedHi all,
Here is an initial patch for the 7.x-2.x branch, a disclaimer though I couldn't test this with a functioning form as i'm seeing errors related to jQuery.validate in my development environment, meaning I can't reproduce this issue (naturally because the JS is falling over). I'd appreciate it if someone who has a working version of the 2.x module, and can reproduce the issue to please test this.
The minimal testing i've done is via using the clientside_validation/captcha URL.
Thanks
Aaron
Comment #54
aaron.ferris commentedIgnore the patch above, it had some unwanted changes related to me trying to fix my local issues....
Comment #55
aaron.ferris commented