In 52910, we introduced a cron key that prevents cron from executing from remote sites that don't know the key.

However, there is no upgrade path for this for existing sites, and hence using cron.php?cron_key=drupal would work, defeating the whole purpose of this change.

This patch introduces an update for update.php generation of the cron key, so existing sites are protected.

I tested it and it works ... More tests appreciated.

CommentFileSizeAuthor
#3 235821-2.patch1.22 KBkbahey
#1 235821.patch786 byteskbahey
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kbahey’s picture

kbahey CreditAttribution: kbahey commented
FileSize
235821.patch786 bytes

And here is the patch

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented

I'd at least make it mt_rand() or some such rather than time() for the hash

kbahey’s picture

kbahey CreditAttribution: kbahey commented
FileSize
235821-2.patch1.22 KB

This reroll changes the value used in the hash to be mt_rand(), instead of just time(), since it can be guessed by brute force.

Thanks to pwolanin for this idea.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Needs review » Fixed

I've committed this patch to CVS HEAD. Thanks!

breyten’s picture

breyten CreditAttribution: breyten commented
Status: Fixed » Needs review

If we use mt_rand() here, we should reroll #52910: Restrict access to cron as well to use the same.

breyten’s picture

breyten CreditAttribution: breyten commented

*coughs* nevermind me, sorry!

birdmanx35’s picture

birdmanx35 CreditAttribution: birdmanx35 commented
Status: Needs review » Fixed
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

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