marcrobledo / RomPatcher.js

An IPS/UPS/APS/BPS/RUP/PPF/xdelta ROM patcher made in HTML5.
https://www.marcrobledo.com/RomPatcher.js/
Other
712 stars 141 forks source link

Add inputMd5 #78

Closed xenophile127 closed 1 week ago

xenophile127 commented 2 weeks ago

Checking roms is done with inputCrc32, but CRC32 is little used these days due to chances of collisions--mostly due to the ease of generating intentional collisions, but people are still moving away from them.

While SHA-1 seems to be the golden standard for security MD5 seems to be the format used by far most often by people involved in retro gaming.

Adding inputMd5 would allow hiding the CRC32 field and using just MD5s for everything.

SolidLamp commented 2 weeks ago

SHA-1 espically and MD5 to an extent also suffer from collision generation. It would be more appropriate to use one of SHA-2's variants (SHA256, SHA512, etc.) which are very popular to verify the integrity of software.

xenophile127 commented 2 weeks ago

@SolidLamp, adding additional hashes could be a separate issue. My motivation for an inputMd5 is to match what I perceive as the current status quo--sites and tools seem to have mostly abandoned CRC32 for identifying roms and often include only MD5.

Collision generation seems like a less academic issue for non-rom based software.

SolidLamp commented 2 weeks ago

@xenophile127 I agree. While I would say that SHA-2 is superior, MD5 is an improvement over CRC32. Therefore, I think that, although SHA-2 could potentially future-proof the software for a new scenario, we should prioritise MD5 for hashing the ROMs; as you have shown, MD5 is well enough currently.

marcrobledo commented 2 weeks ago

While it is true that both CRC32 and MD5 can have collisions, I don't think this is an issue in ROM patching. The chances of an user providing an invalid ROM that has a collision are probably zero. Let's say a obscure NES game have a collision with a Sega Genesis game, it's unlikely someone will provide that NES ROM while trying to apply a Genesis patch. Even if the collision was between two ROMs of the same system, it's almost impossible that they were the same game (but different revision, headers...).

Anyway, added inputMd5 to the embeded patch system :-) Keep in mind that Javascript max number limit wouldn't allow a MD5 passed as a number, so you need to provide it as a string such as 'cafebabecafebabecafebabecafebabe'.

SHA-1 validation would be a little harder to implement due to SHA-1 calculation being done via Web Crypto API and using promises. On the other hand CRC32 and MD5 are calculated using custom code (since Web Crypto cannot do them). While it wouldn't be impossible, I don't think it would be worth, as I said, we can trust both CRC32 and MD5 in our case.

marcrobledo commented 1 week ago

@xenophile127 have you tested it by any chance?

xenophile127 commented 1 week ago

Tested successfully with two patches, each using one or two of the same md5s.