Honeypot timelimits depend on a form value delivered by the form submitter. Attached patch signs the timestamp to prevent tampering by the submitter.

CommentFileSizeAuthor
honepot_signed_timestamps.patch2.85 KBHeine
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

geerlingguy’s picture

This makes sense to me; I think I had an internal TODO to look into doing this way back when I was first building the module... and completely forgot about it. The 7.x patch looks good, but I also need to get it into 8.x first, so would you (or anyone else) be willing to work on the port soon? I'll try to get to it soon, but this summer will be a little busy for me.

  • geerlingguy committed 5e2a03a on 7.x-1.x authored by Heine
    Issue #2525760 by Heine, geerlingguy: Sign timestamp values
    
geerlingguy’s picture

Status: Needs review » Patch (to be ported)

@Heine - This works great! I've made one slight change, using a default $honeypot_time value of 0 (int) instead of FALSE (bool), so the function always returns an int. It works just the same with the validation logic, but makes one fewer function in the world with mixed return values :)

Next up: porting to D8.

geerlingguy’s picture

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

  • geerlingguy committed 0e1ef2c on 8.x-1.x
    Issue #2525760 by geerlingguy: Sign timestamp values
    
geerlingguy’s picture

Status: Patch (to be ported) » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

justAChris’s picture

Status: Closed (fixed) » Needs work

Looks like we lost a "!" in "!$honeypot_time" on commit for the 8.x-1.x branch.

On commit 0e1ef2c: (not on the current patch)

   // Make sure current time - (time_limit + form time value) is greater than 0.
   // If not, throw an error.
-  if (time() < ($honeypot_time + $time_limit)) {
+  if ($honeypot_time || time() < ($honeypot_time + $time_limit)) {
     _honeypot_log($form_state->getValue('form_id'), 'honeypot_time');
     $time_limit = honeypot_get_time_limit();

7.x-1.x branch looks fine

geerlingguy’s picture

Priority: Normal » Major

  • geerlingguy committed 43cd06a on 8.x-1.x
    Issue #2525760 by geerlingguy: Fix _honeypot_time_restriction_validate...
geerlingguy’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.