Cyberchef: Bug report: FromHex then MD5/SHA1 has wrong computation

[slurdge]

Summary

When using FromHex then MD5 computation recipe, sometimes, the hash is wrong.
It seems that bytes >= 0x80 have a problem with the transformation.

Example

See the following example:

The correct hash for a single 0x80 byte should be:

>>> import hashlib
>>> hashlib.md5(bytes([0x80])).hexdigest()
'8d39dd7eef115ea6975446ef4082951f'

Possible solutions

It seems that at least another online tool is failing at this: https://cryptii.com/hexadecimal/md5
Note that the result for SHA1 is also wrong. However, the new version https://v4.cryptii.com/enigma outputs the correct result.

[slurdge]

I have the same result if I do FromBase -> FromDecimal -> MD5

https://gchq.github.io/CyberChef/#recipe=From_Hex(%27Space%27)MD5()&input=ODA
https://gchq.github.io/CyberChef/#recipe=From_Hex(%270x%27)MD5()&input=MHg4MA
https://gchq.github.io/CyberChef/#recipe=From_Base(16)From_Decimal(%27Space%27)MD5()&input=ODA

[nf404]

This is because update accept message as utf-8 string. U need to use updateFromArray for binary data.

var hash = CryptoApi.hasher('md5'); hash.update('\x80'); hash.finalize().stringify('hex')
// result: "602a4f1ee0d3c4da4fe18df6708ba8e0"

var hash = CryptoApi.hasher('md5'); hash.updateFromArray([0x80]); hash.finalize().stringify('hex')
// result: "8d39dd7eef115ea6975446ef4082951f"

[slurdge]

@nf404 nice catch! I'll try to change the methods and do a pull request (I suppose all hash methods follow the same pattern)!

[slurdge]

I got it to work by changing the MD5 function as follow:

    runMD5: function (input, args) {
        var bytes = [];
        for (var i = 0, msgLen = input.length; i < msgLen; i++) {
            bytes.push(input.charCodeAt(i));
        }
        var hash = CryptoApi.hasher('md5'); hash.updateFromArray(bytes); return hash.finalize().stringify('hex');
    },

Before doing a pull request, do you have any comments ? It seems kind of wasteful to do charCodeAt then fromCharCode inside the hasher lib. An alternative way would be to do as follow:

```javascript
var hash = CryptoApi.hasher('md5');
var state = hash.getState();
state.message = input;
state.length += input.length;
hash.setState(state);
hash.process();
return hash.finalize().stringify('hex');
````
But it breaks encapsulation...

[nf404]

Nice way. U can use it, because in future version updateFromArray will be removed. Arrays too slow for hashing.

state.message = input;
state.length += input.length;

[nf404]

Also you can just do this:

        var hash = CryptoApi.hasher('md5');
        hash.state.message = input;
        hash.state.length += input.length;
        hash.process();
        return hash.finalize().stringify('hex');

[n1474335]

@nf404 Thanks for identifying the issue here and letting us know that the updateFromArray method will be removed in future.

@slurdge If you want to submit a pull request updating all the hashers that would be really great. Rather than converting all the strings to bytes, it's much easier if you just change the inputType from string to byteArray in src/core/config/OperationConfig.js for each of the operations in question. The input argument will then be a byte array.

[slurdge]

@n1474335 In order to use the byteArray, I would need to use the updateFromArray method. Is that alright with you, even if we need to change it afterwards? The other option is that we continue to use string and modify the state manually as above.

[slurdge]

I made a pull request that factorizes all calls. For now I use the manual string method but we can use updateFromArray or any other method rather easily.

[n1474335]

@slurdge Ah yes, I misunderstood. If the manual state update method is for strings then go ahead and leave it as a string. Make sure you run grunt test once you've made the changes in case some of the hashing tests break. They shouldn't as I generated the expected hashes using Python and some other online sources, however those other sources might also be wrong in the same way.