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

  1. Change the 'type' of the honeypot_time field to 'value' instead of 'hidden', and move the validation in _honeypot_time_restriction_validate() into a global form validate function instead of an #element_validate function.
  2. 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.

Comments

j0rd’s picture

Issue summary: View changes

Added bit about defeating rate limit.

geerlingguy’s picture

Title: Honeypot is easily discoverable, thus easily solved by spammers. » Make honeypot harder to autodetect

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.

Definitely 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).

"Leave this field blank" title in $honeypot_element.

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.

The attached CSS is also detectable

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 :(

j0rd’s picture

"Leave this field blank" title in $honeypot_element.

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.

I 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.

j0rd’s picture

Issue summary: View changes

Mentioned CSS improvements.

geerlingguy’s picture

Version: 7.x-1.13 » 7.x-1.x-dev

I'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.

Jeff Burnz’s picture

FYI 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.

geerlingguy’s picture

Good to know. I really need to test things with screen readers more often, instead of hoping for the best :)

dakala’s picture

No 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.

deggertsen’s picture

@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.'),

dakala’s picture

@deggersten: I'll take another look at the patch and re-roll it. Thanks.

geerlingguy’s picture

Status: Active » Needs work
geerlingguy’s picture

Issue summary: View changes

Updated with simpler issue summary.

bennybobw’s picture

Status: Needs work » Needs review
StatusFileSize
new6.19 KB

We 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:

  1. Allow more than one honeypot element name and randomize if more than one
  2. Allow more than one honeypot description and randomize if more than one
  3. Option to add 'required' class to the honeypot text field
  4. Option to randomize the placement of the honeypot field
bennybobw’s picture

geerlingguy’s picture

A couple questions (before I can get time to fully/manually review the changes):

  1. Not sure, but don't some JS libraries pick up the required class and would throw an error even if a hidden field had the class and wasn't filled in. If that's the case, just make sure the required field isn't enabled by default, and we include a warning in the description text for the setting!
  2. Will this work for people updating the module? For example, if I have a custom honeypot name already, will that be picked up as the first item in the multivalue field?
mxt’s picture

Any 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

geerlingguy’s picture

Status: Needs review » Postponed (maintainer needs more info)

I 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.

bennybobw’s picture

Status: Postponed (maintainer needs more info) » Needs work

Oh 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.

vinmassaro’s picture

I'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.

andy inman’s picture

A 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.

larowlan’s picture

  1. +++ b/honeypot.admin.inc
    @@ -167,8 +185,11 @@ function honeypot_admin_form_validate($form, &$form_state) {
    +  $element_names = explode("\n", $form_state['values']['honeypot_element_name']);
    +  foreach ($element_names as $name) {
    +    if (!preg_match("/^[-_a-zA-Z0-9]+$/", trim($name))) {
    +      form_set_error('honeypot_element_name', t("The element name cannot contain spaces or other special characters."));
    +    }
    

    This 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.

  2. +++ b/honeypot.module
    @@ -183,16 +183,68 @@ function honeypot_add_form_protection(&$form, &$form_state, $options = array())
    +    //if there's more than one element name option, randomize the name
    ...
    +    //Get the element description
    ...
    +    //if there's more than one description, randomize the description
    ...
    +    //Try to randomize element placement
    ...
    +        //only get weights of displayed elements
    ...
    +          //add unique weights to array
    ...
    +    //Add required class to form element to try to fool spam bots
    

    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.

  3. +++ b/honeypot.module
    @@ -183,16 +183,68 @@ function honeypot_add_form_protection(&$form, &$form_state, $options = array())
    +      $honeypot_element = trim($honeypot_elements[$rand]);
    

    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;

  4. +++ b/honeypot.module
    @@ -183,16 +183,68 @@ function honeypot_add_form_protection(&$form, &$form_state, $options = array())
    +      $honeypot_element = trim($honeypot_elements[0]);
    

    Using reset() here might be more robust

  5. +++ b/honeypot.module
    @@ -183,16 +183,68 @@ function honeypot_add_form_protection(&$form, &$form_state, $options = array())
    +      '#title' => $honeypot_description,
    +      '#description' => $honeypot_description,
    

    You need check_plain() here, or at very least filter_xss/filter_xss_admin - otherwise you're introducing an XSS exploit

Status: Needs work » Needs review
idebr’s picture

Status: Needs review » Needs work
crystaldawn’s picture

What 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.

geerlingguy’s picture

@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.

geerlingguy’s picture

Adding related issue: #2525760: Sign timestamp values.

bennybobw’s picture

@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.

attisan’s picture

StatusFileSize
new7.18 KB

added css class-creation support

attisan’s picture

Status: Needs work » Needs review
Breakerandi’s picture

I am not able to apply such a long patch, can we please put this into the d7 dev version? PLEASE!

geerlingguy’s picture

Status: Needs review » Closed (won't fix)

@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:

  1. Add a potentially-randomizable honeypot field description.
  2. Add a potentially-randomizable form weight placement (so the element would appear in a different spot in a given form).
  3. Add the ability to add a 'required' class to honeypot fields.

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.

geerlingguy’s picture

Also, 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).

Hakaku’s picture

If 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).

sin@’s picture

Hi
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.

hkirsman’s picture

Why was this issue closed?

crystaldawn’s picture

I 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.