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());
Files: 
CommentFileSizeAuthor
#1 1884828-1.patch3.64 KBpwolanin
PASSED: [[SimpleTest]]: [MySQL] 49,607 pass(es).
[ View ]

Comments

StatusFileSize
new3.64 KB
PASSED: [[SimpleTest]]: [MySQL] 49,607 pass(es).
[ View ]

Also, rand() is slow and unreliable - mt_rand() should be used instead per the PHP docs.

Status:Active» Needs review

Assigned:Unassigned» pwolanin

Issue tags:+VDC

Tagging.

Status:Needs review» Reviewed & tested by the community

Perfect, and I don't think performance matters here as it's not executed really often.

+1 for mt_rand()

Status:Reviewed & tested by the community» Fixed

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

Anyone know whether this might be worth to backport to D7?

Yes, I would consider this for backport - ideally all 7.x modules should have stopped using md5.

Project:Drupal core» Views
Version:8.x-dev» 7.x-3.x-dev
Component:views.module» Code
Status:Fixed» Patch (to be ported)

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

@merlinofchaos in the block patch at #1884826: Regression - replace md5 in Block module calls with sha2 hashes I did:

substr(hash('sha256', $delta), 0, 32);

though happily that code was't needed at all.

Probably better (more bits) is:

$hash = substr(drupal_hash_base64($delta), 0, 32);

Which we use in facetapi for block deltas:
http://drupalcode.org/project/facetapi.git/blob/refs/heads/7.x-1.x:/face...

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

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