plan2net / webp

Create a WebP copy for images (TYPO3 CMS)
GNU General Public License v3.0
60 stars 34 forks source link

Not enough entropy for GeneralUtility::shortMD5 #9

Closed DanielRuf closed 5 years ago

DanielRuf commented 5 years ago

shortMD5 can be problematic in multiple ways.

I would recommend to use crc32 or a better algorithm like SHA1 (full hash) and use the full entropy.

DanielRuf commented 5 years ago

https://forge.typo3.org/issues/54730.html

MD5 hashes !== checksums and there is no real performance difference if this field is an index in the database table. Also char always uses / allocates 10 bytes (more in mb4 and so on) while varchar does not (up to).

The described performance advantages are only valid for MyISAM tables and dafabase server settings with very low memory and pool settings for the caches (recent versions are highly optimized and the difference is not very noticable).

DanielRuf commented 5 years ago

I'll check if it makes sense to reimplement sha1sum in pure PHP (using SplFile).

Anyway, are there currently good reasons to not use the full hash?

wazum commented 5 years ago

The good reason is, that the 'checksum' field is defined by the core extension as char(10) and any larger hash would not be saved here.

Please file a bug report on forge.typo3.org to tackle the issue where it belongs (in the core extension) and link the mentioned issue (#54730) from above. Thanks!

DanielRuf commented 5 years ago

The good reason is, that the 'checksum' field is defined by the core extension as char(10) and any larger hash would not be saved here.

But there is internally no check for the length.

Ok, will do so then. But still, MD5 is a very weak hashing algorithm and it is already broken (we can easily calculate collissions) ;-)

And this is my point here (use a full SHA1 hash or better). But if this is not possible without the core changes I'll open the issue in Forge.