Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Per Drupal secure coding standards at http://drupal.org/node/845876
md5 and sha1 should not be used any place in Drupal core since 7.0, but are re-introduced in the Views module for 8.x:
core/modules/views/lib/Drupal/views/Plugin/views/cache/CachePluginBase.php
298: $this->resultsKey = $this->view->storage->get('name') . ':' . $this->displayHandler->display['id'] . ':results:' . md5(serialize($key_data));
322: $this->outputKey = $this->view->storage->get('name') . ':' . $this->displayHandler->display['id'] . ':output:' . md5(serialize($key_data));
core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayPluginBase.php
133: $cid = 'unpackOptions:' . md5(serialize(array($this->options, $options)));
core/modules/views/lib/Drupal/views/Plugin/views/style/StylePluginBase.php
559: $grouping = md5(serialize($grouping));
core/modules/views/lib/Drupal/views/ViewExecutable.php
1416: $this->dom_id = !empty($this->dom_id) ? $this->dom_id : md5($this->storage->get('name') . REQUEST_TIME . rand());
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 1884828-1.patch | 3.64 KB | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedAlso, rand() is slow and unreliable - mt_rand() should be used instead per the PHP docs.
Comment #2
pwolanin CreditAttribution: pwolanin commentedComment #3
pwolanin CreditAttribution: pwolanin commentedComment #4
tim.plunkettTagging.
Comment #5
dawehnerPerfect, and I don't think performance matters here as it's not executed really often.
+1 for mt_rand()
Comment #6
webchickI always found these kinds of changes to just protect Drupal from brain-dead security scanners that even flag "md5" in *comments* and *text files* as security issues kinda silly, but OTOH I can attest that the sec. team gets bogus bug reports about this all the time, so it's nice to eliminate these. Thanks!
Committed and pushed to 8.x. Thanks!
Comment #7
dawehnerAnyone know whether this might be worth to backport to D7?
Comment #8
pwolanin CreditAttribution: pwolanin commentedYes, I would consider this for backport - ideally all 7.x modules should have stopped using md5.
Comment #9
dawehnerComment #10
merlinofchaos CreditAttribution: merlinofchaos commentedIn at least some cases in D7, the 32 byte length of the hash is the reason we're using it, not any security reason; sha256 keys are longer, I believe, so can't be used in that situation. If md5 is really to be considered evil (and boy, I dislike that) we would need to come up with another solution.
Comment #11
pwolanin CreditAttribution: pwolanin commented@merlinofchaos in the block patch at #1884826: Regression - replace md5 in Block module calls with sha2 hashes I did:
though happily that code was't needed at all.
Probably better (more bits) is:
Which we use in facetapi for block deltas:
http://drupalcode.org/project/facetapi.git/blob/refs/heads/7.x-1.x:/face...
Comment #12
merlinofchaos CreditAttribution: merlinofchaos commentedAn update should also clear caches and other things where we're using these MD5s for later lookups to avoid confusion.
Also changing those will unexpectedly change people's block deltas. While it seems highly unlikely that anyone is using a silly md5 for their CSS on their view blocks...and yet, I've seen people do weirder things. So we could potentially hurt those sites.
Comment #13
dawehnerThat is a good point!
Another point is that the blocks are currently stored in the block table with it's md5-deltas, so if you would use sha2 the block positions for example
would get lost without an update function, but as blocks are even stored in features you simply don't have a way to fix all of them.
Comment #14
gregglesI found this recently when searching and just wanted to update at least the title to say sha256 since that is what I believe is the goal and not just any of the sha2 functions (see this explainer about the differences).
Comment #15
DamienMcKenna