Problem/Motivation
There are a few things that Honeypot does that make it easy for a competent and persistent spammer to automatically detect what he'll need to do to successfully submit spam without getting caught by Honeypot, such as finding the 'honeypot_time' value in the form's HTML and calculating how long to wait before submitting the form.
Some of these things may be able to be changed to further strengthen a site against persistent spammers.
Proposed resolution
- Change the 'type' of the
honeypot_timefield to 'value' instead of 'hidden', and move the validation in_honeypot_time_restriction_validate()into a global form validate function instead of an#element_validatefunction. - The 'Leave this field blank' text for the honeypot element field should be configurable, or possible able to be randomized, to prevent easy detection of the field based on the label.
Original report by j0rd
If you enable time_restriction in honey pot, a hidden value is added into the form
<input type="hidden" name="honeypot_time" value="1352194940">
Any spammer could check for this value, attempt to avoid the default honeypot field name, stall 5 seconds and submit the form.
Any spammer could also edit this value to be a long time in the past, thus defeating the rate limitation.
Could we not change this field from 'type' => 'hidden' to 'type' => 'value', thus it doesn't show up in the HTML and still passes the value. If you're going to change from type hidden to type value, you're not longer able to use #element_validate and will need to use the global validator.
----
Same goes for "Leave this field blank" title in $honeypot_element. This should be configurable (based off honeypot_element, turning "-" "_" into spaces and using uc_words to uc_first would make me happy.
----
The attached CSS is also detectable, but since it has other value uses like in combination with javascript, it would hardly be noticeable by your casual spammer.
I've personally removed that, and buried it into my CSS files.
| Comment | File | Size | Author |
|---|---|---|---|
| #25 | honeypot-detection-patch-1833192-25.patch | 7.18 KB | attisan |
| #10 | honeypot-detection-patch-1833192-10.patch | 6.19 KB | bennybobw |
| #6 | honeypot-element-description-1833192.patch.txt | 1.3 KB | dakala |
Comments
Comment #0.0
j0rd commentedAdded bit about defeating rate limit.
Comment #1
geerlingguy commentedDefinitely worth trying. I don't remember why I used a hidden field instead of a value, but I'll have to run some tests and make sure the value method works just the same. (Might've had to do with caching).
Definitely would be good to have this be configurable—the main thing is, we want to make sure that blind/screenreader users, or users on sites where the CSS is not applying (as with some sites that have poorly configured files directories) don't fill out the field.
Also, merging in #1833148: Easy to bypass with this request, as they're quite related.
Quite true, but I've found that there are very few spambots set up with this level of detection. More often than not, by this time, Honeypot methods won't be working well for you anyways, and you'll probably have switched to a much more involved service/customized process. For example, Drupal.org uses this module along with some other custom techniques to combat spam—this module does the grunt work to combat 80% of spammers, but there are other things in place to try to combat the rest. Even with a massive effort, some spammers will get through :(
Comment #2
j0rd commentedI understand why this was done, but unfortunately the majority of "blind/screenreaders" are spambots these days. You can't have it both ways unfortunately. The key is to just make sure it's not detectable between two sites, thus you can't write a blanket spambot to avoid honeypot. Ideally it would be semi-randomized on a single site, but I don't think this is much less important.
Thanks for your feedback and taking my points into consideration.
Comment #2.0
j0rd commentedMentioned CSS improvements.
Comment #3
geerlingguy commentedI've updated the issue summary with a bit more concise/concrete text. I'm planning on trying to fix the first two problems; I'll leave the CSS as is.
Comment #4
Jeff Burnz commentedFYI since .homepage-textfield uses display none it's not enunciated by screen readers (nor is it focusable) unless by some freak incident they have CSS disabled in their browser, IME this is unlikely, so that case at least shouldn't really be a worry.
Comment #5
geerlingguy commentedGood to know. I really need to test things with screen readers more often, instead of hoping for the best :)
Comment #6
dakalaNo harm done if there's a configuration setting for the honeypot textfield description "Please leave this field blank" I think. This simple patch adds this feature. Thanks.
Comment #7
deggertsen commented@dakala, thanks for your patch, it seems that that change is pretty common sense. Shouldn't the label/title be changed as well though?
'#title' => variable_get('honeypot_element_description', 'Leave this field blank.'),Comment #8
dakala@deggersten: I'll take another look at the patch and re-roll it. Thanks.
Comment #9
geerlingguy commentedComment #9.0
geerlingguy commentedUpdated with simpler issue summary.
Comment #10
bennybobw commentedWe were having some trouble with spam still getting through on one of our sites. Here's a first attempt at implementing some new features.
Patch adds the following:
Comment #11
bennybobw commented10: honeypot-detection-patch-1833192-10.patch queued for re-testing.
Comment #12
geerlingguy commentedA couple questions (before I can get time to fully/manually review the changes):
Comment #13
mxtAny news on this?
My site is constantly under spam attack and honeypot is no longer effective as in the past: it is now completely bypassed on registration from spiders from [mail]@itregi.com.
Thank you very much for making honeypot better
Comment #14
geerlingguy commentedI still don't have time for a full review, but if someone could answer the questions in #12, that would help speed this patch along. I'd really like to include it soon, though.
Comment #15
bennybobw commentedOh sorry to be slow on this, it didn't show up in my inbox.
So all of the techniques in the above patch failed to stop the spam signups on a couple of our sites. We ended up going with requiring the browser to make an ajax request in the background when the page loads to validate our forms. That stopped the spam since the bots don't seem to have javascript support (yet).
I think the patch should probably be scrapped and we should come up with new techniques. Required fields and randomizing the honeypot field name/description really did nothing.
Comment #16
vinmassaro commentedI'm also interested in this topic. We are trying to find a spam prevention method that doesn't disable caching. A client of ours wants a webform in a block, on the home page, which is very susceptible to spam, and we're currently using CAPTCHA/reCAPTCHA modules which disable page caching.
@bennybobw: it's unfortunate the patch did not work for you. I'm surprised the honeypot field renaming was not effective.
Comment #17
andy inman commentedA couple of ideas:
Why not have several honeypot fields rather than just one? An entry in any of them could be considered spam. Maybe there are some spambots who will try randomly leaving fields blank? (I would try that if I wanted to write a spambot!). If there were several fields (with suitable names) it would be hard for a spammer to detect HoneyPot and similar methods.
Again, if I wanted to write a spambot, I might try to checking CSS rules for the input field. If I find one set to display: none than I would not put any entry in it.
Comment #18
larowlanThis is an important piece of the code that prevents against admin-lead XSS exploits. Given we're touching it here, we should be adding new tests to verify it still works in a multi-value entry.
Nitpick: these don't meet coding standards (see drupal.org/node/1354). Should have a space after //, should start with capital letter, should end in a full-stop.
When using the random name, this should be added to the $form for other modules that wish to alter the honeypot element, otherwise they have no performant way of determining the key. Something like
$form['#honeypot_element'] = $honeypot_element;Using reset() here might be more robust
You need check_plain() here, or at very least filter_xss/filter_xss_admin - otherwise you're introducing an XSS exploit
Comment #20
idebr commentedComment #21
crystaldawn commentedWhat is the status of this? I'm rather tired of having to install my custom module to fix such a trivial issue that should be in the core of honeypot? This needs to be a configurable item just like the field element name itself, end of discussion. Not sure what all the discussion about this is all about, TLDR but it's a very simple change. Every field/filename/etc that honeypot injects needs to be configurable (and forces the user to change them to non-obvious names like not allowing the word honey in any of it) and that will end this discussion completely. I have a module that does all of this but it's pointless to contribute it as long as this issue queue is active which I dont know if it is or not.
Comment #22
geerlingguy commented@crystaldawn - I assure you, this module's development is still very active :)
To be clear, for this issue to progress any further, the questions in #12 need to be answered, and the code review changes suggested in #18 need to be addressed.
Currently, I don't have the time to answer these questions, do these changes, and add reliable tests for the changes on my own. And while I do see some benefit from this issue's changes, I don't think changes will make as big an impact on Honeypot's effectiveness as some people hope, so I'm not prioritizing the work here over work to make sure, for example, the D8 port is ready to go whenever D8 is released.
Comment #23
geerlingguy commentedAdding related issue: #2525760: Sign timestamp values.
Comment #24
bennybobw commented@crystaldawn and @geerlingguy in #15 I mentioned that we tried implementing these features on some of our sites and didn't see any reduction in the amount of spam. So I'm not sure it's worth pursuing.
The thing that did work was having the browser make an ajax request to the server on page load that returns a token that the form requires in order to submit. Apparently most of these bots still don't have an javascript capability.
Comment #25
attisanadded css class-creation support
Comment #26
attisanComment #27
Breakerandi commentedI am not able to apply such a long patch, can we please put this into the d7 dev version? PLEASE!
Comment #28
geerlingguy commented@Breakerandi et all - Please see (and address) comments in #12, #18, #22, and #24. Once that is addressed, I'll consider adding some (or maybe all) of the changes introduced in this patch so far. It will also need tests, and will need to be ported to D8, so the simpler the better.
Thinking about this further, I'd rather not add this entire 'bundle of changes' in one patch, because it's much easier for me to review, add tests for, and analyze the effectiveness of, each change individually. It looks like this patch currently does the following:
Additionally, there are parts of the code that aren't following Drupal coding standards, and there are no tests for the functionality. I would much rather look at the merits of each of the above changes separately, in separate issues—and like @bennybobw said in #24, I have also tried some of these changes and they don't make a big (or most of the time, any) difference. I'm more interested in keeping things simple (the 80%+ use case) and finding ways to optimize what we already have (like maybe instead of adding more admin config stuff, make it so the weight is randomized per form generation...).
But again, I'd love to work on each of these new feature additions in a separate issue, so I'm going to close this for now.
Thank you so much for your work, and I'm sorry to ask you to do a little more, but I think to find resolution, this is the best option.
Comment #29
geerlingguy commentedAlso, as a follow up, in the time since this issue has been created, there have been at least 3-4 other small improvements that have made their way in by having small, tested changes (tests aren't a requirement for me to review the change, but I don't want to merge anything without a test, especially in the D8/D7 versions).
Comment #30
Hakaku commentedIf the issue regarding the string "Leave this field blank" is addressed, please also make sure that it's translatable (if it's customizable or has a preset string, and not just randomized).
Comment #31
sin@ commentedHi
sorry if you have already issued a patch for this but I recently updated honeypot which was working flawless and now we are getting spam again. There was also this confusing 'leave this field blank' field showing, I cleared cache and it went away not sure if it will show again later! Is there any way to make sure that is hidden? I have set the time limit to 0 and I have a name for that 'element name' field, but don't know what was the deal with this new field on the screen and why after update we are getting spam (not as many as we used to without honeypot but we were at 0 spam for a year with honeypot until yesterday that I let it update to 7.x-1.22). Thanks.
Comment #32
hkirsman commentedWhy was this issue closed?
Comment #33
crystaldawn commentedI have no idea. I havent used this module in about 3 years because it's long been defeated by most bots specifically because of this issue. Until it's fixed, imo, the module is completely useless. I didnt notice someone had closed it. It should remain open until fixed and possibly increased to "major" since there have been bots in the wild that circumvent it due to this problem. I now use Recaptcha for anything spam related, it cannot be beat in it's success rate.